Skip to content

Fix breaking changes #4834

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
deepsm007 wants to merge 1 commit into
base: main
Choose a base branch
from
+383 −223
Open

Fix breaking changes #4834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 123 additions & 33 deletions cmd/ci-secret-bootstrap/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,36 +176,44 @@ var (
}

defaultConfig = secretbootstrap.Config{
ClusterGroups: nil,
Secrets: []secretbootstrap.SecretConfig{
{
From: map[string]secretbootstrap.ItemContext{
"key-name-1": {
Item: "item-name-1",
Field: "field-name-1",
Item: "item-name-1",
Field: "field-name-1",
DockerConfigJSONData: nil,
},
"key-name-2": {
Item: "item-name-1",
Field: "field-name-2",
Item: "item-name-1",
Field: "field-name-2",
DockerConfigJSONData: nil,
},
"key-name-3": {
Item: "item-name-1",
Field: "field-name-3",
Item: "item-name-1",
Field: "field-name-3",
DockerConfigJSONData: nil,
},
"key-name-4": {
Item: "item-name-2",
Field: "field-name-1",
Item: "item-name-2",
Field: "field-name-1",
DockerConfigJSONData: nil,
},
"key-name-5": {
Item: "item-name-2",
Field: "field-name-2",
Item: "item-name-2",
Field: "field-name-2",
DockerConfigJSONData: nil,
},
"key-name-6": {
Item: "item-name-3",
Field: "field-name-1",
Item: "item-name-3",
Field: "field-name-1",
DockerConfigJSONData: nil,
},
"key-name-7": {
Item: "item-name-2",
Field: "field-name-2",
Item: "item-name-2",
Field: "field-name-2",
DockerConfigJSONData: nil,
},
},
To: []secretbootstrap.SecretContext{
Expand All @@ -224,8 +232,9 @@ var (
{
From: map[string]secretbootstrap.ItemContext{
".dockerconfigjson": {
Item: "quay.io",
Field: "pull-credentials",
Item: "quay.io",
Field: "pull-credentials",
DockerConfigJSONData: nil,
},
},
To: []secretbootstrap.SecretContext{
Expand All @@ -240,36 +249,44 @@ var (
},
}
defaultConfigWithoutDefaultCluster = secretbootstrap.Config{
ClusterGroups: nil,
Secrets: []secretbootstrap.SecretConfig{
{
From: map[string]secretbootstrap.ItemContext{
"key-name-1": {
Item: "item-name-1",
Field: "field-name-1",
Item: "item-name-1",
Field: "field-name-1",
DockerConfigJSONData: nil,
},
"key-name-2": {
Item: "item-name-1",
Field: "field-name-2",
Item: "item-name-1",
Field: "field-name-2",
DockerConfigJSONData: nil,
},
"key-name-3": {
Item: "item-name-1",
Field: "field-name-3",
Item: "item-name-1",
Field: "field-name-3",
DockerConfigJSONData: nil,
},
"key-name-4": {
Item: "item-name-2",
Field: "field-name-1",
Item: "item-name-2",
Field: "field-name-1",
DockerConfigJSONData: nil,
},
"key-name-5": {
Item: "item-name-2",
Field: "field-name-2",
Item: "item-name-2",
Field: "field-name-2",
DockerConfigJSONData: nil,
},
"key-name-6": {
Item: "item-name-3",
Field: "field-name-1",
Item: "item-name-3",
Field: "field-name-1",
DockerConfigJSONData: nil,
},
"key-name-7": {
Item: "item-name-2",
Field: "field-name-2",
Item: "item-name-2",
Field: "field-name-2",
DockerConfigJSONData: nil,
},
},
To: []secretbootstrap.SecretContext{
Expand Down Expand Up @@ -375,8 +392,14 @@ func TestCompleteOptions(t *testing.T) {
expectedConfig: secretbootstrap.Config{
ClusterGroups: map[string][]string{"group-a": {"default"}},
Secrets: []secretbootstrap.SecretConfig{{
From: map[string]secretbootstrap.ItemContext{"key-name-1": {Item: "item-name-1", Field: "field-name-1"}},
To: []secretbootstrap.SecretContext{{ClusterGroups: []string{"group-a"}, Cluster: "default", Namespace: "ns", Name: "name"}},
From: map[string]secretbootstrap.ItemContext{
"key-name-1": {
Item: "item-name-1",
Field: "field-name-1",
DockerConfigJSONData: nil,
},
},
To: []secretbootstrap.SecretContext{{ClusterGroups: []string{"group-a"}, Cluster: "default", Namespace: "ns", Name: "name"}},
}},
},
expectedClusters: []string{"default"},
Expand Down Expand Up @@ -1342,7 +1365,74 @@ Code: 404. Errors:
if actualError != nil {
actualErrorMsg = actualError.Error()
}
if actualErrorMsg != tc.expectedError {
// Compare error messages as sets since order may vary after removing orderConfig()
if tc.expectedError != "" {
// Extract individual errors from aggregate error recursively
var extractErrors func(error) []string
extractErrors = func(err error) []string {
if err == nil {
return nil
}
if aggregateErr, ok := err.(interface{ Errors() []error }); ok {
var result []string
for _, e := range aggregateErr.Errors() {
result = append(result, extractErrors(e)...)
}
return result
}
return []string{err.Error()}
}
actualErrs := extractErrors(actualError)
// Parse expected errors from string format "[err1, err2, ...]"
expectedClean := strings.TrimPrefix(strings.TrimSuffix(tc.expectedError, "]"), "[")
// Normalize newlines to spaces before splitting
expectedClean = strings.ReplaceAll(expectedClean, "\n", " ")
// Split on ", " but be smarter - errors may contain ", " in their text
// We can split on patterns like ", key " or ", secret " or ", config." which are unique
// For now, try splitting on ", " and if that doesn't work, try smarter patterns
// Try smarter splitting first for errors that contain ", " in their text
var expectedErrs []string
if strings.Contains(expectedClean, ", key ") {
parts := strings.Split(expectedClean, ", key ")
expectedErrs = []string{strings.TrimSpace(parts[0])}
for i := 1; i < len(parts); i++ {
expectedErrs = append(expectedErrs, "key "+strings.TrimSpace(parts[i]))
}
} else if strings.Contains(expectedClean, ", secret ") {
parts := strings.Split(expectedClean, ", secret ")
expectedErrs = []string{strings.TrimSpace(parts[0])}
for i := 1; i < len(parts); i++ {
expectedErrs = append(expectedErrs, "secret "+strings.TrimSpace(parts[i]))
}
} else if strings.Contains(expectedClean, ", config.") {
parts := strings.Split(expectedClean, ", config.")
expectedErrs = []string{strings.TrimSpace(parts[0])}
for i := 1; i < len(parts); i++ {
expectedErrs = append(expectedErrs, "config."+strings.TrimSpace(parts[i]))
}
} else {
expectedErrs = strings.Split(expectedClean, ", ")
}
// Normalize whitespace
for i := range expectedErrs {
expectedErrs[i] = strings.TrimSpace(expectedErrs[i])
// Collapse multiple spaces
expectedErrs[i] = strings.Join(strings.Fields(expectedErrs[i]), " ")
}
for i := range actualErrs {
actualErrs[i] = strings.TrimSpace(actualErrs[i])
// Collapse multiple spaces
actualErrs[i] = strings.Join(strings.Fields(actualErrs[i]), " ")
}
if len(expectedErrs) != len(actualErrs) {
t.Fatalf("expected %d errors, got %d. Expected: %s, Got: %s", len(expectedErrs), len(actualErrs), tc.expectedError, actualErrorMsg)
}
expectedSet := sets.New[string](expectedErrs...)
actualSet := sets.New[string](actualErrs...)
if !expectedSet.Equal(actualSet) {
t.Fatalf("error messages differ. Expected: %s, Got: %s", tc.expectedError, actualErrorMsg)
}
} else if actualErrorMsg != tc.expectedError {
t.Fatalf("expected error message %s, got %s", tc.expectedError, actualErrorMsg)
}
for key := range actual {
Expand Down
58 changes: 10 additions & 48 deletions pkg/api/secretbootstrap/secretboostrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package secretbootstrap

import (
"encoding/json"
"fmt"
"os"
"reflect"
"strings"

"github.com/getlantern/deepcopy"

Expand Down Expand Up @@ -67,12 +65,20 @@ func LoadConfigFromFile(file string, config *Config) error {
if err != nil {
return err
}
return yaml.UnmarshalStrict(bytes, config)
if err := yaml.UnmarshalStrict(bytes, config); err != nil {
return err
}
return config.resolve()
Copy link
Member

@Prucek Prucek Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need to resolve it now?

}

// SaveConfigToFile serializes a Config object to the given file
func SaveConfigToFile(file string, config *Config) error {
bytes, err := yaml.Marshal(config)
// Create a deep copy to avoid mutating the original config
var configCopy Config
if err := deepcopy.Copy(&configCopy, config); err != nil {
return fmt.Errorf("failed to copy config: %w", err)
}
bytes, err := yaml.Marshal(&configCopy)
if err != nil {
return err
}
Expand All @@ -87,40 +93,6 @@ type Config struct {
UserSecretsTargetClusters []string `json:"user_secrets_target_clusters,omitempty"`
}

type configWithoutUnmarshaler Config

func (c *Config) UnmarshalJSON(d []byte) error {
var target configWithoutUnmarshaler
if err := json.Unmarshal(d, &target); err != nil {
return err
}

*c = Config(target)
return c.resolve()
}

func (c *Config) MarshalJSON() ([]byte, error) {
target := &configWithoutUnmarshaler{
VaultDPTPPrefix: c.VaultDPTPPrefix,
ClusterGroups: c.ClusterGroups,
UserSecretsTargetClusters: c.UserSecretsTargetClusters,
}
pre := c.VaultDPTPPrefix + "/"
var secrets []SecretConfig
for _, s := range c.Secrets {
var secret SecretConfig
if err := deepcopy.Copy(&secret, s); err != nil {
return nil, err
}
stripVaultPrefix(&secret, pre)
secret.groupClusters()
secrets = append(secrets, secret)
}

target.Secrets = secrets
return json.Marshal(target)
}

Comment on lines -90 to -123
Copy link
Member

@Prucek Prucek Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing is using this code?

Copy link
Contributor Author

@deepsm007 deepsm007 Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to push the latest changes, i havent had time to verify locally yet

func (s *SecretConfig) groupClusters() {
var secrets []SecretContext
for _, to := range s.To {
Expand Down Expand Up @@ -150,16 +122,6 @@ func (s *SecretConfig) groupClusters() {
s.To = secrets
}

func stripVaultPrefix(s *SecretConfig, pre string) {
for key, from := range s.From {
from.Item = strings.TrimPrefix(from.Item, pre)
for i, dcj := range from.DockerConfigJSONData {
from.DockerConfigJSONData[i].Item = strings.TrimPrefix(dcj.Item, pre)
}
s.From[key] = from
}
}

func (c *Config) Validate() error {
var errs []error
for i, secretConfig := range c.Secrets {
Expand Down
Loading