Skip to content

Commit

Permalink
Refactor configutil and fix bug with catalog parsing
Browse files Browse the repository at this point in the history
This will refactor configutil to pass the validators
in sync config function so that defaulting and validation can
be performed separately

default validators are exposed to be used by deps

catalog parsing was broken as soon as it find the other key
for same catalog so fixed that

refactored internal catalog parsing to have index data in map
to convert the strut back to configmap

added utility to convert struct back to configmap to be
used by deps and added unit tests
  • Loading branch information
piyush-garg authored and chmouel committed Jun 10, 2024
1 parent 3f9bcd0 commit 17aae4b
Show file tree
Hide file tree
Showing 16 changed files with 336 additions and 66 deletions.
2 changes: 2 additions & 0 deletions hack/dev/kind/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ function configure_pac() {
kubectl patch configmap -n pipelines-as-code -p "{\"data\":{\"tekton-dashboard-url\": \"http://dashboard.${DOMAIN_NAME}\"}}" --type merge pipelines-as-code
# add custom catalog so we can use it in e2e, this will points to the normal upstream hub so we can easily use it
kubectl patch configmap -n pipelines-as-code -p '{"data":{"catalog-1-id": "custom", "catalog-1-name": "tekton", "catalog-1-url": "https://api.hub.tekton.dev/v1"}}' --type merge pipelines-as-code
# add one more custom catalog so we can use it in e2e for multiple catalog support, this will points to the normal upstream hub so we can easily use it
kubectl patch configmap -n pipelines-as-code -p '{"data":{"catalog-2-id": "custom2", "catalog-2-name": "tekton", "catalog-2-url": "https://api.hub.tekton.dev/v1"}}' --type merge pipelines-as-code
set +x
if [[ -n ${PAC_PASS_SECRET_FOLDER} ]]; then
echo "Installing PAC secrets"
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/tknpac/resolve/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func Command(run *params.Run, streams *cli.IOStreams) *cobra.Command {
return fmt.Errorf("you need to at least specify a file with -f")
}

if err := settings.SyncConfig(run.Clients.Log, &run.Info.Pac.Settings, map[string]string{}); err != nil {
if err := settings.SyncConfig(run.Clients.Log, &run.Info.Pac.Settings, map[string]string{}, settings.DefaultValidators()); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/consoleui/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func NewCustomConsole(pacInfo *info.PacOpts) *CustomConsole {

func (o *CustomConsole) GetName() string {
if o.pacInfo.CustomConsoleName == "" {
return "Not configured"
return fmt.Sprintf("https://url.setting.%s.is.not.configured", settings.CustomConsoleNameKey)
}
return o.pacInfo.CustomConsoleName
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/consoleui/custom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func TestCustomBad(t *testing.T) {
Name: "pr",
},
}
assert.Assert(t, strings.Contains(c.GetName(), "Not configured"))
assert.Assert(t, strings.Contains(c.GetName(), "is.not.configured"))
assert.Assert(t, strings.Contains(c.URL(), "is.not.configured"))
assert.Assert(t, strings.Contains(c.DetailURL(pr), "is.not.configured"))
assert.Assert(t, strings.Contains(c.TaskLogURL(pr, nil), "is.not.configured"))
Expand Down
12 changes: 6 additions & 6 deletions pkg/hub/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ func TestGetTask(t *testing.T) {
var hubCatalogs sync.Map
hubCatalogs.Store(
"default", settings.HubCatalog{
ID: "default",
URL: testHubURL,
Name: testCatalogHubName,
Index: "default",
URL: testHubURL,
Name: testCatalogHubName,
})
hubCatalogs.Store(
"anotherHub", settings.HubCatalog{
ID: "anotherHub",
URL: testHubURL,
Name: testCatalogHubName,
Index: "1",
URL: testHubURL,
Name: testCatalogHubName,
})
tests := []struct {
name string
Expand Down
24 changes: 12 additions & 12 deletions pkg/matcher/annotation_tasks_install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ func TestRemoteTasksGetTaskFromAnnotations(t *testing.T) {
var hubCatalogs sync.Map
hubCatalogs.Store(
"default", settings.HubCatalog{
ID: "default",
URL: testHubURL,
Name: testCatalogHubName,
Index: "default",
URL: testHubURL,
Name: testCatalogHubName,
})
hubCatalogs.Store(
"anotherHub", settings.HubCatalog{
ID: "anotherHub",
URL: testHubURL,
Name: testCatalogHubName,
Index: "1",
URL: testHubURL,
Name: testCatalogHubName,
})
tests := []struct {
annotations map[string]string
Expand Down Expand Up @@ -304,15 +304,15 @@ func TestGetPipelineFromAnnotations(t *testing.T) {
var hubCatalogs sync.Map
hubCatalogs.Store(
"default", settings.HubCatalog{
ID: "default",
URL: testHubURL,
Name: testCatalogHubName,
Index: "default",
URL: testHubURL,
Name: testCatalogHubName,
})
hubCatalogs.Store(
"anotherHub", settings.HubCatalog{
ID: "anotherHub",
URL: testHubURL,
Name: testCatalogHubName,
Index: "1",
URL: testHubURL,
Name: testCatalogHubName,
})
tests := []struct {
annotations map[string]string
Expand Down
2 changes: 1 addition & 1 deletion pkg/params/info/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (i *Info) UpdatePacOpts(logger *zap.SugaredLogger, configData map[string]st
i.pacMutex.Lock()
defer i.pacMutex.Unlock()

if err := settings.SyncConfig(logger, &i.Pac.Settings, configData); err != nil {
if err := settings.SyncConfig(logger, &i.Pac.Settings, configData, settings.DefaultValidators()); err != nil {
return nil, err
}
return &i.Pac.Settings, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/params/info/info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ func TestNewInfo(t *testing.T) {

catalog, ok := value.(settings.HubCatalog)
assert.Equal(t, true, ok)
assert.Equal(t, catalog.ID, "default")
assert.Equal(t, catalog.Index, "default")
assert.Equal(t, catalog.Name, settings.HubCatalogNameDefaultValue)
}
34 changes: 16 additions & 18 deletions pkg/params/settings/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ var (
)

type HubCatalog struct {
ID string
Name string
URL string
Index string
Name string
URL string
}

// if there is a change performed on the default value,
Expand Down Expand Up @@ -80,33 +80,31 @@ func DefaultSettings() Settings {
newSettings := &Settings{}
hubCatalog := &sync.Map{}
hubCatalog.Store("default", HubCatalog{
ID: "default",
Name: HubCatalogNameDefaultValue,
URL: HubURLDefaultValue,
Index: "default",
Name: HubCatalogNameDefaultValue,
URL: HubURLDefaultValue,
})
newSettings.HubCatalogs = hubCatalog

_ = configutil.ValidateAndAssignValues(nil, map[string]string{}, newSettings, map[string]func(string) error{
"ErrorDetectionSimpleRegexp": isValidRegex,
"TektonDashboardURL": isValidURL,
"CustomConsoleURL": isValidURL,
"CustomConsolePRTaskLog": startWithHTTPorHTTPS,
"CustomConsolePRDetail": startWithHTTPorHTTPS,
}, false)
_ = configutil.ValidateAndAssignValues(nil, map[string]string{}, newSettings, map[string]func(string) error{}, false)

return *newSettings
}

func SyncConfig(logger *zap.SugaredLogger, setting *Settings, config map[string]string) error {
setting.HubCatalogs = getHubCatalogs(logger, setting.HubCatalogs, config)

err := configutil.ValidateAndAssignValues(logger, config, setting, map[string]func(string) error{
func DefaultValidators() map[string]func(string) error {
return map[string]func(string) error{
"ErrorDetectionSimpleRegexp": isValidRegex,
"TektonDashboardURL": isValidURL,
"CustomConsoleURL": isValidURL,
"CustomConsolePRTaskLog": startWithHTTPorHTTPS,
"CustomConsolePRDetail": startWithHTTPorHTTPS,
}, true)
}
}

func SyncConfig(logger *zap.SugaredLogger, setting *Settings, config map[string]string, validators map[string]func(string) error) error {
setting.HubCatalogs = getHubCatalogs(logger, setting.HubCatalogs, config)

err := configutil.ValidateAndAssignValues(logger, config, setting, validators, true)
if err != nil {
return fmt.Errorf("failed to validate and assign values: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/params/settings/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func TestSyncConfig(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
var test Settings

err := SyncConfig(logger, &test, tc.configMap)
err := SyncConfig(logger, &test, tc.configMap, DefaultValidators())

// set hub catalogs to nil to avoid comparison error
// test separately
Expand All @@ -166,5 +166,5 @@ func TestDefaultSettings(t *testing.T) {
assert.Assert(t, ok)
catalog, ok := catalogValue.(HubCatalog)
assert.Assert(t, ok)
assert.Equal(t, catalog.ID, "default")
assert.Equal(t, catalog.Index, "default")
}
65 changes: 65 additions & 0 deletions pkg/params/settings/convert.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package settings

import (
"fmt"
"reflect"
"strconv"
"strings"
"sync"
)

func ConvertPacStructToConfigMap(settings *Settings) map[string]string {
config := map[string]string{}
if settings == nil {
return config
}
structValue := reflect.ValueOf(settings).Elem()
structType := reflect.TypeOf(settings).Elem()

for i := 0; i < structType.NumField(); i++ {
field := structType.Field(i)
fieldName := field.Name

jsonTag := field.Tag.Get("json")
if jsonTag == "-" {
continue
}
key := strings.ToLower(jsonTag)
element := structValue.FieldByName(fieldName)
if !element.IsValid() {
continue
}

//nolint
switch field.Type.Kind() {
case reflect.String:
config[key] = element.String()
case reflect.Bool:
config[key] = strconv.FormatBool(element.Bool())
case reflect.Int:
config[key] = strconv.FormatInt(element.Int(), 10)
case reflect.Ptr:
// for hub catalogs map
if key == "" {
data := element.Interface().(*sync.Map)
data.Range(func(key, value interface{}) bool {
catalogData := value.(HubCatalog)
if key == "default" {
config[HubURLKey] = catalogData.URL
config[HubCatalogNameKey] = catalogData.Name
return true
}
config[fmt.Sprintf("%s-%s-%s", "catalog", catalogData.Index, "id")] = key.(string)
config[fmt.Sprintf("%s-%s-%s", "catalog", catalogData.Index, "name")] = catalogData.Name
config[fmt.Sprintf("%s-%s-%s", "catalog", catalogData.Index, "url")] = catalogData.URL
return true
})
}
default:
// Skip unsupported field types
continue
}
}

return config
}
Loading

0 comments on commit 17aae4b

Please sign in to comment.