From 0cc00ae026fb929385091f1de1e4ad5a2c6a82c9 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Fri, 16 May 2025 08:47:35 -0400 Subject: [PATCH 1/3] coverage --- api/v1alpha1/blueprint_types.go | 12 +- api/v1alpha1/blueprint_types_test.go | 419 +++++++++++++++++++++++++++ 2 files changed, 427 insertions(+), 4 deletions(-) diff --git a/api/v1alpha1/blueprint_types.go b/api/v1alpha1/blueprint_types.go index 3d96962b2..ca5c40731 100644 --- a/api/v1alpha1/blueprint_types.go +++ b/api/v1alpha1/blueprint_types.go @@ -313,13 +313,17 @@ func (b *Blueprint) Merge(overlay *Blueprint) { // Merge the Reference type inline, prioritizing the first non-empty field if overlay.Repository.Ref.Commit != "" { b.Repository.Ref.Commit = overlay.Repository.Ref.Commit - } else if overlay.Repository.Ref.Name != "" { + } + if overlay.Repository.Ref.Name != "" { b.Repository.Ref.Name = overlay.Repository.Ref.Name - } else if overlay.Repository.Ref.SemVer != "" { + } + if overlay.Repository.Ref.SemVer != "" { b.Repository.Ref.SemVer = overlay.Repository.Ref.SemVer - } else if overlay.Repository.Ref.Tag != "" { + } + if overlay.Repository.Ref.Tag != "" { b.Repository.Ref.Tag = overlay.Repository.Ref.Tag - } else if overlay.Repository.Ref.Branch != "" { + } + if overlay.Repository.Ref.Branch != "" { b.Repository.Ref.Branch = overlay.Repository.Ref.Branch } diff --git a/api/v1alpha1/blueprint_types_test.go b/api/v1alpha1/blueprint_types_test.go index 7d9f5ea06..f23a37302 100644 --- a/api/v1alpha1/blueprint_types_test.go +++ b/api/v1alpha1/blueprint_types_test.go @@ -396,6 +396,425 @@ func TestBlueprint_Merge(t *testing.T) { }) } }) + + t.Run("OverlayComponentWithDifferentSource", func(t *testing.T) { + base := &Blueprint{ + TerraformComponents: []TerraformComponent{{Path: "mod", Source: "A"}}, + } + overlay := &Blueprint{ + TerraformComponents: []TerraformComponent{{Path: "mod", Source: "B"}}, + } + base.Merge(overlay) + if len(base.TerraformComponents) != 1 || base.TerraformComponents[0].Source != "B" { + t.Errorf("expected overlay component with Source B, got %v", base.TerraformComponents) + } + }) + + t.Run("OverlayComponentWithNewPath", func(t *testing.T) { + base := &Blueprint{ + TerraformComponents: []TerraformComponent{{Path: "mod1", Source: "A"}}, + } + overlay := &Blueprint{ + TerraformComponents: []TerraformComponent{{Path: "mod2", Source: "B"}}, + } + base.Merge(overlay) + if len(base.TerraformComponents) != 1 || base.TerraformComponents[0].Path != "mod2" { + t.Errorf("expected overlay component with Path mod2, got %v", base.TerraformComponents) + } + }) + + t.Run("OverlaySourceWithNewName", func(t *testing.T) { + base := &Blueprint{ + Sources: []Source{{Name: "A", Url: "urlA"}}, + } + overlay := &Blueprint{ + Sources: []Source{{Name: "B", Url: "urlB"}}, + } + base.Merge(overlay) + foundB := false + for _, s := range base.Sources { + if s.Name == "B" && s.Url == "urlB" { + foundB = true + } + } + if !foundB { + t.Errorf("expected overlay source with Name B, got %v", base.Sources) + } + }) + + t.Run("OverlayRepositoryRefFields", func(t *testing.T) { + base := &Blueprint{ + Repository: Repository{ + Ref: Reference{ + Branch: "main", + }, + }, + } + overlay := &Blueprint{ + Repository: Repository{ + Ref: Reference{ + Commit: "abc123", + Name: "v1", + SemVer: "1.0.0", + Tag: "v1.0.0", + Branch: "develop", + }, + }, + } + base.Merge(overlay) + if base.Repository.Ref.Commit != "abc123" { + t.Errorf("Expected Repository.Ref.Commit to be 'abc123', but got '%s'", base.Repository.Ref.Commit) + } + if base.Repository.Ref.Name != "v1" { + t.Errorf("Expected Repository.Ref.Name to be 'v1', but got '%s'", base.Repository.Ref.Name) + } + if base.Repository.Ref.SemVer != "1.0.0" { + t.Errorf("Expected Repository.Ref.SemVer to be '1.0.0', but got '%s'", base.Repository.Ref.SemVer) + } + if base.Repository.Ref.Tag != "v1.0.0" { + t.Errorf("Expected Repository.Ref.Tag to be 'v1.0.0', but got '%s'", base.Repository.Ref.Tag) + } + if base.Repository.Ref.Branch != "develop" { + t.Errorf("Expected Repository.Ref.Branch to be 'develop', but got '%s'", base.Repository.Ref.Branch) + } + }) + + t.Run("OverlayNonEmptyKustomizations", func(t *testing.T) { + base := &Blueprint{ + Kustomizations: []Kustomization{{Name: "A"}}, + } + overlay := &Blueprint{ + Kustomizations: []Kustomization{{Name: "B"}}, + } + base.Merge(overlay) + if len(base.Kustomizations) != 1 || base.Kustomizations[0].Name != "B" { + t.Errorf("expected overlay kustomization with Name B, got %v", base.Kustomizations) + } + }) + + t.Run("OverlayWithEmptyFields", func(t *testing.T) { + base := &Blueprint{ + Kind: "Blueprint", + ApiVersion: "v1alpha1", + Metadata: Metadata{ + Name: "original", + Description: "original description", + Authors: []string{"author1"}, + }, + Repository: Repository{ + Url: "http://example.com/repo1", + Ref: Reference{ + Branch: "main", + }, + }, + Sources: []Source{ + { + Name: "source1", + Url: "http://example.com/source1", + PathPrefix: "prefix1", + Ref: Reference{ + Branch: "main", + }, + }, + }, + TerraformComponents: []TerraformComponent{ + { + Source: "source1", + Path: "module/path1", + Values: map[string]any{"key1": "value1"}, + FullPath: "original/full/path", + Destroy: ptrBool(true), + }, + }, + Kustomizations: []Kustomization{ + { + Name: "kustomization1", + Path: "kustomize/path1", + Components: []string{"component1"}, + PostBuild: &PostBuild{ + Substitute: map[string]string{"key1": "value1"}, + SubstituteFrom: []SubstituteReference{ + {Kind: "ConfigMap", Name: "config1"}, + }, + }, + }, + }, + } + overlay := &Blueprint{} + base.Merge(overlay) + if base.Kind != "Blueprint" { + t.Errorf("Expected Kind to be 'Blueprint', but got '%s'", base.Kind) + } + if base.ApiVersion != "v1alpha1" { + t.Errorf("Expected ApiVersion to be 'v1alpha1', but got '%s'", base.ApiVersion) + } + if base.Metadata.Name != "original" { + t.Errorf("Expected Metadata.Name to be 'original', but got '%s'", base.Metadata.Name) + } + if base.Repository.Url != "http://example.com/repo1" { + t.Errorf("Expected Repository.Url to be 'http://example.com/repo1', but got '%s'", base.Repository.Url) + } + if base.Repository.Ref.Branch != "main" { + t.Errorf("Expected Repository.Ref.Branch to be 'main', but got '%s'", base.Repository.Ref.Branch) + } + if len(base.Sources) != 1 || base.Sources[0].Name != "source1" { + t.Errorf("Expected Sources to contain 'source1', but got %v", base.Sources) + } + if len(base.TerraformComponents) != 1 || base.TerraformComponents[0].Path != "module/path1" { + t.Errorf("Expected TerraformComponents to contain 'module/path1', but got %v", base.TerraformComponents) + } + if len(base.Kustomizations) != 1 || base.Kustomizations[0].Name != "kustomization1" { + t.Errorf("Expected Kustomizations to contain 'kustomization1', but got %v", base.Kustomizations) + } + }) + + t.Run("OverlayWithEmptySlices", func(t *testing.T) { + base := &Blueprint{ + Kind: "Blueprint", + ApiVersion: "v1alpha1", + Metadata: Metadata{ + Name: "original", + Description: "original description", + Authors: []string{"author1"}, + }, + Repository: Repository{ + Url: "http://example.com/repo1", + Ref: Reference{ + Branch: "main", + }, + }, + Sources: []Source{ + { + Name: "source1", + Url: "http://example.com/source1", + PathPrefix: "prefix1", + Ref: Reference{ + Branch: "main", + }, + }, + }, + TerraformComponents: []TerraformComponent{ + { + Source: "source1", + Path: "module/path1", + Values: map[string]any{"key1": "value1"}, + FullPath: "original/full/path", + Destroy: ptrBool(true), + }, + }, + Kustomizations: []Kustomization{ + { + Name: "kustomization1", + Path: "kustomize/path1", + Components: []string{"component1"}, + PostBuild: &PostBuild{ + Substitute: map[string]string{"key1": "value1"}, + SubstituteFrom: []SubstituteReference{ + {Kind: "ConfigMap", Name: "config1"}, + }, + }, + }, + }, + } + overlay := &Blueprint{ + Sources: []Source{}, + TerraformComponents: []TerraformComponent{}, + Kustomizations: []Kustomization{}, + } + base.Merge(overlay) + if len(base.Sources) != 1 || base.Sources[0].Name != "source1" { + t.Errorf("Expected Sources to contain 'source1', but got %v", base.Sources) + } + if len(base.TerraformComponents) != 1 || base.TerraformComponents[0].Path != "module/path1" { + t.Errorf("Expected TerraformComponents to contain 'module/path1', but got %v", base.TerraformComponents) + } + if len(base.Kustomizations) != 1 || base.Kustomizations[0].Name != "kustomization1" { + t.Errorf("Expected Kustomizations to contain 'kustomization1', but got %v", base.Kustomizations) + } + }) + + t.Run("OverlayWithRepositoryRefFields", func(t *testing.T) { + base := &Blueprint{ + Repository: Repository{ + Ref: Reference{ + Branch: "main", + }, + }, + } + overlay := &Blueprint{ + Repository: Repository{ + Ref: Reference{ + Commit: "abc123", + Name: "v1", + SemVer: "1.0.0", + Tag: "v1.0.0", + Branch: "develop", + }, + }, + } + base.Merge(overlay) + if base.Repository.Ref.Commit != "abc123" { + t.Errorf("Expected Repository.Ref.Commit to be 'abc123', but got '%s'", base.Repository.Ref.Commit) + } + if base.Repository.Ref.Name != "v1" { + t.Errorf("Expected Repository.Ref.Name to be 'v1', but got '%s'", base.Repository.Ref.Name) + } + if base.Repository.Ref.SemVer != "1.0.0" { + t.Errorf("Expected Repository.Ref.SemVer to be '1.0.0', but got '%s'", base.Repository.Ref.SemVer) + } + if base.Repository.Ref.Tag != "v1.0.0" { + t.Errorf("Expected Repository.Ref.Tag to be 'v1.0.0', but got '%s'", base.Repository.Ref.Tag) + } + if base.Repository.Ref.Branch != "develop" { + t.Errorf("Expected Repository.Ref.Branch to be 'develop', but got '%s'", base.Repository.Ref.Branch) + } + }) + + t.Run("OverlayWithEmptyRepositorySecretName", func(t *testing.T) { + base := &Blueprint{ + Repository: Repository{ + SecretName: "base-secret", + }, + } + overlay := &Blueprint{ + Repository: Repository{ + SecretName: "", + }, + } + base.Merge(overlay) + if base.Repository.SecretName != "base-secret" { + t.Errorf("Expected Repository.SecretName to be 'base-secret', but got '%s'", base.Repository.SecretName) + } + }) + + t.Run("OverlayWithEmptySourceName", func(t *testing.T) { + base := &Blueprint{ + Sources: []Source{ + { + Name: "source1", + Url: "http://example.com/source1", + PathPrefix: "prefix1", + Ref: Reference{ + Branch: "main", + }, + }, + }, + } + overlay := &Blueprint{ + Sources: []Source{ + { + Name: "", + Url: "http://example.com/source2", + PathPrefix: "prefix2", + Ref: Reference{ + Branch: "feature", + }, + }, + }, + } + base.Merge(overlay) + if len(base.Sources) != 1 || base.Sources[0].Name != "source1" { + t.Errorf("Expected Sources to contain 'source1', but got %v", base.Sources) + } + }) + + t.Run("OverlayWithNoMatchingTerraformComponent", func(t *testing.T) { + base := &Blueprint{ + TerraformComponents: []TerraformComponent{ + { + Source: "source1", + Path: "module/path1", + Values: map[string]any{"key1": "value1"}, + FullPath: "original/full/path", + Destroy: ptrBool(true), + }, + }, + } + overlay := &Blueprint{ + TerraformComponents: []TerraformComponent{ + { + Source: "source2", + Path: "module/path2", + Values: map[string]any{"key2": "value2"}, + FullPath: "overlay/full/path", + Destroy: ptrBool(false), + }, + }, + } + base.Merge(overlay) + if len(base.TerraformComponents) != 1 || base.TerraformComponents[0].Path != "module/path2" { + t.Errorf("Expected TerraformComponents to contain 'module/path2', but got %v", base.TerraformComponents) + } + }) + + t.Run("OverlayWithSamePathDifferentSource", func(t *testing.T) { + base := &Blueprint{ + TerraformComponents: []TerraformComponent{ + { + Source: "source1", + Path: "module/path1", + Values: map[string]any{"key1": "value1"}, + FullPath: "original/full/path", + Destroy: ptrBool(true), + }, + }, + } + overlay := &Blueprint{ + TerraformComponents: []TerraformComponent{ + { + Source: "source2", + Path: "module/path1", + Values: map[string]any{"key2": "value2"}, + FullPath: "overlay/full/path", + Destroy: ptrBool(false), + }, + }, + } + base.Merge(overlay) + if len(base.TerraformComponents) != 1 || base.TerraformComponents[0].Source != "source2" { + t.Errorf("Expected TerraformComponents to contain 'source2', but got %v", base.TerraformComponents) + } + }) + + t.Run("OverlayWithEmptyKustomizations", func(t *testing.T) { + base := &Blueprint{ + Kustomizations: []Kustomization{{Path: "kustomize"}}, + } + overlay := &Blueprint{ + Kustomizations: []Kustomization{}, + } + base.Merge(overlay) + if len(base.Kustomizations) != 1 || base.Kustomizations[0].Path != "kustomize" { + t.Errorf("expected base kustomizations to be retained, got %v", base.Kustomizations) + } + }) + + t.Run("OverlayWithNonEmptyKind", func(t *testing.T) { + base := &Blueprint{Kind: "base-kind"} + overlay := &Blueprint{Kind: "new-kind"} + base.Merge(overlay) + if base.Kind != "new-kind" { + t.Errorf("expected Kind to be overwritten to 'new-kind', got %v", base.Kind) + } + }) + + t.Run("OverlayWithNonEmptyApiVersion", func(t *testing.T) { + base := &Blueprint{ApiVersion: "v1"} + overlay := &Blueprint{ApiVersion: "v2"} + base.Merge(overlay) + if base.ApiVersion != "v2" { + t.Errorf("expected ApiVersion to be overwritten to 'v2', got %v", base.ApiVersion) + } + }) + + t.Run("OverlayWithNonEmptyRepositorySecretName", func(t *testing.T) { + base := &Blueprint{Repository: Repository{SecretName: "base-secret"}} + overlay := &Blueprint{Repository: Repository{SecretName: "new-secret"}} + base.Merge(overlay) + if base.Repository.SecretName != "new-secret" { + t.Errorf("expected Repository.SecretName to be overwritten to 'new-secret', got %v", base.Repository.SecretName) + } + }) } func TestBlueprint_DeepCopy(t *testing.T) { From bd69df42b570273c00f600b6eadf156e54d3fbf2 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Fri, 16 May 2025 08:47:56 -0400 Subject: [PATCH 2/3] Add --wait flag --- cmd/install.go | 13 +- cmd/install_test.go | 32 ++ cmd/up.go | 8 + cmd/up_test.go | 25 ++ pkg/blueprint/blueprint_handler.go | 439 +++++++++++++++++++----- pkg/blueprint/blueprint_handler_test.go | 312 +++++++++++++++++ pkg/blueprint/mock_blueprint_handler.go | 9 + pkg/constants/constants.go | 5 + 8 files changed, 756 insertions(+), 87 deletions(-) diff --git a/cmd/install.go b/cmd/install.go index 9113d4d75..b7fd9b070 100644 --- a/cmd/install.go +++ b/cmd/install.go @@ -8,8 +8,9 @@ import ( ) var installCmd = &cobra.Command{ - Use: "install", - Short: "Install the blueprint's cluster-level services", + Use: "install", + Short: "Install the blueprint's cluster-level services", + SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { controller := cmd.Context().Value(controllerKey).(ctrl.Controller) @@ -59,10 +60,18 @@ var installCmd = &cobra.Command{ return fmt.Errorf("Error installing blueprint: %w", err) } + // If wait flag is set, wait for kustomizations to be ready + if waitFlag { + if err := blueprintHandler.WaitForKustomizations(); err != nil { + return fmt.Errorf("failed waiting for kustomizations: %w", err) + } + } + return nil }, } func init() { + installCmd.Flags().BoolVar(&waitFlag, "wait", false, "Wait for kustomization resources to be ready") rootCmd.AddCommand(installCmd) } diff --git a/cmd/install_test.go b/cmd/install_test.go index 5842b988c..7611b71e5 100644 --- a/cmd/install_test.go +++ b/cmd/install_test.go @@ -190,4 +190,36 @@ func TestInstallCmd(t *testing.T) { t.Errorf("Expected error %q, got %q", expectedError, err.Error()) } }) + + t.Run("WaitFlag_CallsWaitForKustomizations", func(t *testing.T) { + // Given a set of mocks with proper configuration + mocks := setupInstallMocks(t, nil) + + // Set waitFlag = true + waitFlag = true + defer func() { waitFlag = false }() + + // Set up a flag to check if WaitForKustomizations is called + called := false + mocks.BlueprintHandler.WaitForKustomizationsFunc = func() error { + called = true + return nil + } + + // Set up command arguments + rootCmd.SetArgs([]string{"install"}) + + // When executing the command + err := Execute(mocks.Controller) + + // Then no error should occur + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + + // And WaitForKustomizations should have been called + if !called { + t.Error("Expected WaitForKustomizations to be called, but it was not") + } + }) } diff --git a/cmd/up.go b/cmd/up.go index 359fff19e..2231af594 100644 --- a/cmd/up.go +++ b/cmd/up.go @@ -10,6 +10,7 @@ import ( var ( installFlag bool // Declare the install flag + waitFlag bool // Declare the wait flag ) var upCmd = &cobra.Command{ @@ -143,6 +144,12 @@ var upCmd = &cobra.Command{ if err := blueprintHandler.Install(); err != nil { return fmt.Errorf("Error installing blueprint: %w", err) } + // If wait flag is set, poll for kustomization readiness + if waitFlag { + if err := blueprintHandler.WaitForKustomizations(); err != nil { + return fmt.Errorf("Error waiting for kustomizations: %w", err) + } + } } // Indicate successful setup of the Windsor environment. @@ -154,5 +161,6 @@ var upCmd = &cobra.Command{ func init() { upCmd.Flags().BoolVar(&installFlag, "install", false, "Install the blueprint after setting up the environment") + upCmd.Flags().BoolVar(&waitFlag, "wait", false, "Wait for kustomization resources to be ready") rootCmd.AddCommand(upCmd) } diff --git a/cmd/up_test.go b/cmd/up_test.go index 6d2997489..dce0eb3fa 100644 --- a/cmd/up_test.go +++ b/cmd/up_test.go @@ -397,4 +397,29 @@ func TestUpCmd(t *testing.T) { t.Errorf("Expected 'No blueprint handler found', got '%v'", err) } }) + + t.Run("WaitFlag_CallsWaitForKustomizations", func(t *testing.T) { + mocks := setupUpMocks(t) + installFlag = true + waitFlag = true + defer func() { installFlag = false; waitFlag = false }() + + called := false + mocks.BlueprintHandler.WaitForKustomizationsFunc = func() error { + called = true + return nil + } + mocks.BlueprintHandler.InstallFunc = func() error { + return nil + } + + rootCmd.SetArgs([]string{"up", "--install", "--wait"}) + err := Execute(mocks.Controller) + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + if !called { + t.Error("Expected WaitForKustomizations to be called, but it was not") + } + }) } diff --git a/pkg/blueprint/blueprint_handler.go b/pkg/blueprint/blueprint_handler.go index c4e1097dc..e73fdfb52 100644 --- a/pkg/blueprint/blueprint_handler.go +++ b/pkg/blueprint/blueprint_handler.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "reflect" + "slices" "strings" "time" @@ -26,6 +27,8 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" @@ -54,6 +57,7 @@ type BlueprintHandler interface { SetRepository(repository blueprintv1alpha1.Repository) error SetTerraformComponents(terraformComponents []blueprintv1alpha1.TerraformComponent) error SetKustomizations(kustomizations []blueprintv1alpha1.Kustomization) error + WaitForKustomizations() error } //go:embed templates/default.jsonnet @@ -68,14 +72,17 @@ type BaseBlueprintHandler struct { blueprint blueprintv1alpha1.Blueprint projectRoot string shims *Shims + + kustomizationWaitPollInterval time.Duration } // NewBlueprintHandler creates a new instance of BaseBlueprintHandler. // It initializes the handler with the provided dependency injector. func NewBlueprintHandler(injector di.Injector) *BaseBlueprintHandler { return &BaseBlueprintHandler{ - injector: injector, - shims: NewShims(), + injector: injector, + shims: NewShims(), + kustomizationWaitPollInterval: constants.DEFAULT_KUSTOMIZATION_WAIT_POLL_INTERVAL, } } @@ -240,6 +247,63 @@ func (b *BaseBlueprintHandler) WriteConfig(path ...string) error { return nil } +// WaitForKustomizations polls for readiness of all kustomizations with a maximum timeout. +// It uses a spinner to show progress and checks both GitRepository and Kustomization status. +// The timeout is calculated based on the longest dependency path through the kustomizations. +func (b *BaseBlueprintHandler) WaitForKustomizations() error { + message := "⏳ Waiting for kustomizations to be ready" + spin := spinner.New(spinner.CharSets[14], 100*time.Millisecond, spinner.WithColor("green")) + spin.Suffix = " " + message + spin.Start() + defer spin.Stop() + + timeout := time.After(b.calculateMaxWaitTime()) + ticker := time.NewTicker(b.kustomizationWaitPollInterval) + defer ticker.Stop() + + kustomizations := b.GetKustomizations() + names := make([]string, len(kustomizations)) + for i, k := range kustomizations { + names[i] = k.Name + } + + for { + select { + case <-timeout: + spin.Stop() + fmt.Fprintf(os.Stderr, "\033[31m✗\033[0m %s - \033[31mTimeout\033[0m\n", message) + return fmt.Errorf("timeout waiting for kustomizations to be ready") + case <-ticker.C: + kubeconfig := os.Getenv("KUBECONFIG") + if err := checkGitRepositoryStatus(kubeconfig); err != nil { + spin.Stop() + fmt.Fprintf(os.Stderr, "\033[31m✗\033[0m %s - \033[31mFailed\033[0m\n", message) + return fmt.Errorf("git repository error: %w", err) + } + status, err := checkKustomizationStatus(kubeconfig, names) + if err != nil { + spin.Stop() + fmt.Fprintf(os.Stderr, "\033[31m✗\033[0m %s - \033[31mFailed\033[0m\n", message) + return fmt.Errorf("kustomization error: %w", err) + } + + allReady := true + for _, ready := range status { + if !ready { + allReady = false + break + } + } + + if allReady { + spin.Stop() + fmt.Fprintf(os.Stderr, "\033[32m✔\033[0m %s - \033[32mDone\033[0m\n", message) + return nil + } + } + } +} + // Install applies the blueprint's Kubernetes resources to the cluster. It handles GitRepositories // for the main repository and sources, Kustomizations for deployments, and a ConfigMap containing // context-specific configuration. Uses environment KUBECONFIG or falls back to in-cluster config. @@ -685,89 +749,6 @@ func (b *BaseBlueprintHandler) yamlMarshalWithDefinedPaths(v any) ([]byte, error return yamlData, nil } -// ============================================================================= -// Kubernetes Client Operations -// ============================================================================= - -type ResourceOperationConfig struct { - ApiPath string - Namespace string - ResourceName string - ResourceInstanceName string - ResourceObject runtime.Object - ResourceType func() runtime.Object -} - -// NOTE: This is a temporary solution until we've integrated the kube client into our DI system. -// As such, this function is not internally covered by our tests. -// -// kubeClientResourceOperation is a comprehensive function that handles the entire lifecycle of creating a Kubernetes client -// and performing a sequence of operations (Get, Post, Put) on Kubernetes resources. It takes a kubeconfig path and a -// configuration object that specifies the parameters for the operations. -var kubeClientResourceOperation = func(kubeconfigPath string, config ResourceOperationConfig) error { - var kubeConfig *rest.Config - var err error - - if kubeconfigPath != "" { - kubeConfig, err = clientcmd.BuildConfigFromFlags("", kubeconfigPath) - } else { - kubeConfig, err = rest.InClusterConfig() - } - - if err != nil { - return fmt.Errorf("failed to create Kubernetes config: %w", err) - } - - clientset, err := kubernetes.NewForConfig(kubeConfig) - if err != nil { - return fmt.Errorf("failed to create Kubernetes client: %w", err) - } - - restClient := clientset.CoreV1().RESTClient() - backgroundCtx := ctx.Background() - - existingResource := config.ResourceType().(runtime.Object) - err = restClient.Get(). - AbsPath(config.ApiPath). - Namespace(config.Namespace). - Resource(config.ResourceName). - Name(config.ResourceInstanceName). - Do(backgroundCtx). - Into(existingResource) - - if err != nil { - if apierrors.IsNotFound(err) { - if err := restClient.Post(). - AbsPath(config.ApiPath). - Namespace(config.Namespace). - Resource(config.ResourceName). - Body(config.ResourceObject). - Do(backgroundCtx). - Error(); err != nil { - return fmt.Errorf("failed to create resource: %w", err) - } - } else { - return fmt.Errorf("failed to get resource: %w", err) - } - } else { - // Ensure the resourceVersion is set for the update - config.ResourceObject.(metav1.Object).SetResourceVersion(existingResource.(metav1.Object).GetResourceVersion()) - - if err := restClient.Put(). - AbsPath(config.ApiPath). - Namespace(config.Namespace). - Resource(config.ResourceName). - Name(config.ResourceInstanceName). - Body(config.ResourceObject). - Do(backgroundCtx). - Error(); err != nil { - return fmt.Errorf("failed to update resource: %w", err) - } - } - - return nil -} - // applyGitRepository creates or updates a GitRepository resource in the cluster. It normalizes // the repository URL format, configures standard intervals and timeouts, and handles secret // references for private repositories. @@ -949,3 +930,291 @@ func (b *BaseBlueprintHandler) applyConfigMap() error { kubeconfig := os.Getenv("KUBECONFIG") return kubeClientResourceOperation(kubeconfig, config) } + +// calculateMaxWaitTime calculates the maximum wait time needed based on kustomization dependencies. +// It builds a dependency graph and uses DFS to find the longest path through it, accumulating +// timeouts for each kustomization in the path. Returns the total time needed for the longest path. +func (b *BaseBlueprintHandler) calculateMaxWaitTime() time.Duration { + kustomizations := b.GetKustomizations() + if len(kustomizations) == 0 { + return 0 + } + + deps := make(map[string][]string) + timeouts := make(map[string]time.Duration) + for _, k := range kustomizations { + deps[k.Name] = k.DependsOn + if k.Timeout != nil { + timeouts[k.Name] = k.Timeout.Duration + } else { + timeouts[k.Name] = constants.DEFAULT_FLUX_KUSTOMIZATION_TIMEOUT + } + } + + var maxPathTime time.Duration + visited := make(map[string]bool) + path := make([]string, 0) + + var dfs func(name string, currentTime time.Duration) + dfs = func(name string, currentTime time.Duration) { + visited[name] = true + path = append(path, name) + currentTime += timeouts[name] + + if currentTime > maxPathTime { + maxPathTime = currentTime + } + + for _, dep := range deps[name] { + if !visited[dep] { + dfs(dep, currentTime) + } else { + // For circular dependencies, we still want to consider the path + // but we don't want to recurse further + if currentTime+timeouts[dep] > maxPathTime { + maxPathTime = currentTime + timeouts[dep] + } + } + } + + visited[name] = false + path = path[:len(path)-1] + } + + // Start DFS from each root node (nodes with no incoming dependencies) + roots := []string{} + for _, k := range kustomizations { + isRoot := true + for _, other := range kustomizations { + if slices.Contains(other.DependsOn, k.Name) { + isRoot = false + break + } + } + if isRoot { + roots = append(roots, k.Name) + } + } + if len(roots) == 0 { + // No roots found (cycle or all nodes have dependencies), start from every node + for _, k := range kustomizations { + dfs(k.Name, 0) + } + } else { + for _, root := range roots { + dfs(root, 0) + } + } + + return maxPathTime +} + +// ============================================================================= +// Kubernetes Client Operations +// ============================================================================= + +type ResourceOperationConfig struct { + ApiPath string + Namespace string + ResourceName string + ResourceInstanceName string + ResourceObject runtime.Object + ResourceType func() runtime.Object +} + +// NOTE: This is a temporary solution until we've integrated the kube client into our DI system. +// As such, this function is not internally covered by our tests. +// +// kubeClientResourceOperation is a comprehensive function that handles the entire lifecycle of creating a Kubernetes client +// and performing a sequence of operations (Get, Post, Put) on Kubernetes resources. It takes a kubeconfig path and a +// configuration object that specifies the parameters for the operations. +var kubeClientResourceOperation = func(kubeconfigPath string, config ResourceOperationConfig) error { + var kubeConfig *rest.Config + var err error + + if kubeconfigPath != "" { + kubeConfig, err = clientcmd.BuildConfigFromFlags("", kubeconfigPath) + } else { + kubeConfig, err = rest.InClusterConfig() + } + + if err != nil { + return fmt.Errorf("failed to create Kubernetes config: %w", err) + } + + clientset, err := kubernetes.NewForConfig(kubeConfig) + if err != nil { + return fmt.Errorf("failed to create Kubernetes client: %w", err) + } + + restClient := clientset.CoreV1().RESTClient() + backgroundCtx := ctx.Background() + + existingResource := config.ResourceType().(runtime.Object) + err = restClient.Get(). + AbsPath(config.ApiPath). + Namespace(config.Namespace). + Resource(config.ResourceName). + Name(config.ResourceInstanceName). + Do(backgroundCtx). + Into(existingResource) + + if err != nil { + if apierrors.IsNotFound(err) { + if err := restClient.Post(). + AbsPath(config.ApiPath). + Namespace(config.Namespace). + Resource(config.ResourceName). + Body(config.ResourceObject). + Do(backgroundCtx). + Error(); err != nil { + return fmt.Errorf("failed to create resource: %w", err) + } + } else { + return fmt.Errorf("failed to get resource: %w", err) + } + } else { + // Ensure the resourceVersion is set for the update + config.ResourceObject.(metav1.Object).SetResourceVersion(existingResource.(metav1.Object).GetResourceVersion()) + + if err := restClient.Put(). + AbsPath(config.ApiPath). + Namespace(config.Namespace). + Resource(config.ResourceName). + Name(config.ResourceInstanceName). + Body(config.ResourceObject). + Do(backgroundCtx). + Error(); err != nil { + return fmt.Errorf("failed to update resource: %w", err) + } + } + + return nil +} + +// NOTE: This is a temporary solution until we've integrated the kube client into our DI system. +// As such, this function is not internally covered by our tests. +// +// checkKustomizationStatus checks the status of all kustomizations in the cluster by name. +// It returns a map of kustomization names to their readiness (true if ready, false otherwise). +// If any kustomization is missing or has failed, it returns an error. The function queries all +// kustomizations in the default namespace, converts them to typed objects, and inspects their +// status conditions for readiness or failure. It ensures all requested kustomizations are present. +var checkKustomizationStatus = func(kubeconfigPath string, names []string) (map[string]bool, error) { + var kubeConfig *rest.Config + var err error + + if kubeconfigPath != "" { + kubeConfig, err = clientcmd.BuildConfigFromFlags("", kubeconfigPath) + } else { + kubeConfig, err = rest.InClusterConfig() + } + + if err != nil { + return nil, fmt.Errorf("failed to create Kubernetes config: %w", err) + } + + dynamicClient, err := dynamic.NewForConfig(kubeConfig) + if err != nil { + return nil, fmt.Errorf("failed to create dynamic client: %w", err) + } + + gvr := schema.GroupVersionResource{ + Group: "kustomize.toolkit.fluxcd.io", + Version: "v1", + Resource: "kustomizations", + } + + objList, err := dynamicClient.Resource(gvr).Namespace(constants.DEFAULT_FLUX_SYSTEM_NAMESPACE). + List(ctx.Background(), metav1.ListOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to list kustomizations: %w", err) + } + + status := make(map[string]bool) + found := make(map[string]bool) + + for _, obj := range objList.Items { + var kustomizeObj kustomizev1.Kustomization + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &kustomizeObj); err != nil { + return nil, fmt.Errorf("failed to convert kustomization %s: %w", kustomizeObj.Name, err) + } + + found[kustomizeObj.Name] = true + ready := false + for _, condition := range kustomizeObj.Status.Conditions { + if condition.Type == "Ready" { + if condition.Status == "True" { + ready = true + } else if condition.Status == "False" && condition.Reason == "ReconciliationFailed" { + return nil, fmt.Errorf("kustomization %s failed: %s", kustomizeObj.Name, condition.Message) + } + break + } + } + status[kustomizeObj.Name] = ready + } + + for _, name := range names { + if !found[name] { + return nil, fmt.Errorf("kustomization %s not found", name) + } + } + + return status, nil +} + +// NOTE: This is a temporary solution until we've integrated the kube client into our DI system. +// As such, this function is not internally covered by our tests. +// +// checkGitRepositoryStatus checks the status of all GitRepository resources in the cluster. +// It returns an error if any repository is not ready or has failed. The function queries all +// GitRepository resources in the default namespace, converts them to typed objects, and inspects +// their status conditions for readiness or failure. If any repository is not ready, it returns an error +// with the repository name and failure message. +var checkGitRepositoryStatus = func(kubeconfigPath string) error { + var kubeConfig *rest.Config + var err error + + if kubeconfigPath != "" { + kubeConfig, err = clientcmd.BuildConfigFromFlags("", kubeconfigPath) + } else { + kubeConfig, err = rest.InClusterConfig() + } + + if err != nil { + return fmt.Errorf("failed to create Kubernetes config: %w", err) + } + + dynamicClient, err := dynamic.NewForConfig(kubeConfig) + if err != nil { + return fmt.Errorf("failed to create dynamic client: %w", err) + } + + gvr := schema.GroupVersionResource{ + Group: "source.toolkit.fluxcd.io", + Version: "v1", + Resource: "gitrepositories", + } + + objList, err := dynamicClient.Resource(gvr).Namespace(constants.DEFAULT_FLUX_SYSTEM_NAMESPACE). + List(ctx.Background(), metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("failed to list git repositories: %w", err) + } + + for _, obj := range objList.Items { + var gitRepo sourcev1.GitRepository + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &gitRepo); err != nil { + return fmt.Errorf("failed to convert git repository %s: %w", gitRepo.Name, err) + } + + for _, condition := range gitRepo.Status.Conditions { + if condition.Type == "Ready" && condition.Status == "False" { + return fmt.Errorf("%s: %s", gitRepo.Name, condition.Message) + } + } + } + + return nil +} diff --git a/pkg/blueprint/blueprint_handler_test.go b/pkg/blueprint/blueprint_handler_test.go index ecb0abaee..3a4feee50 100644 --- a/pkg/blueprint/blueprint_handler_test.go +++ b/pkg/blueprint/blueprint_handler_test.go @@ -8,6 +8,7 @@ import ( "reflect" "strings" "testing" + "time" "github.com/aws/smithy-go/ptr" kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1" @@ -3635,3 +3636,314 @@ func TestTLACode(t *testing.T) { t.Errorf("expected error containing 'blueprint has no authors', got %v", err) } } + +func TestBaseBlueprintHandler_calculateMaxWaitTime(t *testing.T) { + t.Run("EmptyKustomizations", func(t *testing.T) { + // Given a blueprint handler with no kustomizations + handler := &BaseBlueprintHandler{ + blueprint: blueprintv1alpha1.Blueprint{ + Kustomizations: []blueprintv1alpha1.Kustomization{}, + }, + } + + // When calculating max wait time + waitTime := handler.calculateMaxWaitTime() + + // Then it should return 0 since there are no kustomizations + if waitTime != 0 { + t.Errorf("expected 0 duration, got %v", waitTime) + } + }) + + t.Run("SingleKustomization", func(t *testing.T) { + // Given a blueprint handler with a single kustomization + customTimeout := 2 * time.Minute + handler := &BaseBlueprintHandler{ + blueprint: blueprintv1alpha1.Blueprint{ + Kustomizations: []blueprintv1alpha1.Kustomization{ + { + Name: "test-kustomization", + Timeout: &metav1.Duration{ + Duration: customTimeout, + }, + }, + }, + }, + } + + // When calculating max wait time + waitTime := handler.calculateMaxWaitTime() + + // Then it should return the kustomization's timeout + if waitTime != customTimeout { + t.Errorf("expected timeout %v, got %v", customTimeout, waitTime) + } + }) + + t.Run("LinearDependencies", func(t *testing.T) { + // Given a blueprint handler with linear dependencies + timeout1 := 1 * time.Minute + timeout2 := 2 * time.Minute + timeout3 := 3 * time.Minute + handler := &BaseBlueprintHandler{ + blueprint: blueprintv1alpha1.Blueprint{ + Kustomizations: []blueprintv1alpha1.Kustomization{ + { + Name: "kustomization-1", + Timeout: &metav1.Duration{ + Duration: timeout1, + }, + DependsOn: []string{"kustomization-2"}, + }, + { + Name: "kustomization-2", + Timeout: &metav1.Duration{ + Duration: timeout2, + }, + DependsOn: []string{"kustomization-3"}, + }, + { + Name: "kustomization-3", + Timeout: &metav1.Duration{ + Duration: timeout3, + }, + }, + }, + }, + } + + // When calculating max wait time + waitTime := handler.calculateMaxWaitTime() + + // Then it should return the sum of all timeouts + expectedTime := timeout1 + timeout2 + timeout3 + if waitTime != expectedTime { + t.Errorf("expected timeout %v, got %v", expectedTime, waitTime) + } + }) + + t.Run("BranchingDependencies", func(t *testing.T) { + // Given a blueprint handler with branching dependencies + timeout1 := 1 * time.Minute + timeout2 := 2 * time.Minute + timeout3 := 3 * time.Minute + timeout4 := 4 * time.Minute + handler := &BaseBlueprintHandler{ + blueprint: blueprintv1alpha1.Blueprint{ + Kustomizations: []blueprintv1alpha1.Kustomization{ + { + Name: "kustomization-1", + Timeout: &metav1.Duration{ + Duration: timeout1, + }, + DependsOn: []string{"kustomization-2", "kustomization-3"}, + }, + { + Name: "kustomization-2", + Timeout: &metav1.Duration{ + Duration: timeout2, + }, + DependsOn: []string{"kustomization-4"}, + }, + { + Name: "kustomization-3", + Timeout: &metav1.Duration{ + Duration: timeout3, + }, + DependsOn: []string{"kustomization-4"}, + }, + { + Name: "kustomization-4", + Timeout: &metav1.Duration{ + Duration: timeout4, + }, + }, + }, + }, + } + + // When calculating max wait time + waitTime := handler.calculateMaxWaitTime() + + // Then it should return the longest path (1 -> 3 -> 4) + expectedTime := timeout1 + timeout3 + timeout4 + if waitTime != expectedTime { + t.Errorf("expected timeout %v, got %v", expectedTime, waitTime) + } + }) + + t.Run("CircularDependencies", func(t *testing.T) { + // Given a blueprint handler with circular dependencies + timeout1 := 1 * time.Minute + timeout2 := 2 * time.Minute + timeout3 := 3 * time.Minute + handler := &BaseBlueprintHandler{ + blueprint: blueprintv1alpha1.Blueprint{ + Kustomizations: []blueprintv1alpha1.Kustomization{ + { + Name: "kustomization-1", + Timeout: &metav1.Duration{ + Duration: timeout1, + }, + DependsOn: []string{"kustomization-2"}, + }, + { + Name: "kustomization-2", + Timeout: &metav1.Duration{ + Duration: timeout2, + }, + DependsOn: []string{"kustomization-3"}, + }, + { + Name: "kustomization-3", + Timeout: &metav1.Duration{ + Duration: timeout3, + }, + DependsOn: []string{"kustomization-1"}, + }, + }, + }, + } + + // When calculating max wait time + waitTime := handler.calculateMaxWaitTime() + + // Then it should return the sum of all timeouts in the cycle (1+2+3+3) + expectedTime := timeout1 + timeout2 + timeout3 + timeout3 + if waitTime != expectedTime { + t.Errorf("expected timeout %v, got %v", expectedTime, waitTime) + } + }) +} + +func TestBaseBlueprintHandler_WaitForKustomizations(t *testing.T) { + t.Run("AllKustomizationsReady", func(t *testing.T) { + // Given a blueprint handler with multiple kustomizations that are all ready + handler := &BaseBlueprintHandler{ + blueprint: blueprintv1alpha1.Blueprint{ + Kustomizations: []blueprintv1alpha1.Kustomization{ + {Name: "k1", Timeout: &metav1.Duration{Duration: 100 * time.Millisecond}}, + {Name: "k2", Timeout: &metav1.Duration{Duration: 100 * time.Millisecond}}, + }, + }, + } + handler.kustomizationWaitPollInterval = 10 * time.Millisecond + + // And Git repository and kustomization status checks that return success + origCheckGit := checkGitRepositoryStatus + origCheckKustom := checkKustomizationStatus + defer func() { + checkGitRepositoryStatus = origCheckGit + checkKustomizationStatus = origCheckKustom + }() + checkGitRepositoryStatus = func(string) error { return nil } + checkKustomizationStatus = func(string, []string) (map[string]bool, error) { + return map[string]bool{"k1": true, "k2": true}, nil + } + + // When waiting for kustomizations to be ready + err := handler.WaitForKustomizations() + + // Then no error should be returned + if err != nil { + t.Errorf("expected no error, got %v", err) + } + }) + + t.Run("TimeoutWaitingForKustomizations", func(t *testing.T) { + // Given a blueprint handler with kustomizations that never reach ready state + handler := &BaseBlueprintHandler{ + blueprint: blueprintv1alpha1.Blueprint{ + Kustomizations: []blueprintv1alpha1.Kustomization{ + {Name: "k1", Timeout: &metav1.Duration{Duration: 200 * time.Millisecond}}, + {Name: "k2", Timeout: &metav1.Duration{Duration: 200 * time.Millisecond}}, + }, + }, + } + handler.kustomizationWaitPollInterval = 10 * time.Millisecond + + // And status checks that always return not ready + origCheckGit := checkGitRepositoryStatus + origCheckKustom := checkKustomizationStatus + defer func() { + checkGitRepositoryStatus = origCheckGit + checkKustomizationStatus = origCheckKustom + }() + checkGitRepositoryStatus = func(string) error { return nil } + checkKustomizationStatus = func(string, []string) (map[string]bool, error) { + return map[string]bool{"k1": false, "k2": false}, nil + } + + // When waiting for kustomizations to be ready + err := handler.WaitForKustomizations() + + // Then a timeout error should be returned + if err == nil || !strings.Contains(err.Error(), "timeout waiting for kustomizations") { + t.Errorf("expected timeout error, got %v", err) + } + }) + + t.Run("GitRepositoryStatusError", func(t *testing.T) { + // Given a blueprint handler with a kustomization + handler := &BaseBlueprintHandler{ + blueprint: blueprintv1alpha1.Blueprint{ + Kustomizations: []blueprintv1alpha1.Kustomization{ + {Name: "k1", Timeout: &metav1.Duration{Duration: 100 * time.Millisecond}}, + }, + }, + } + handler.kustomizationWaitPollInterval = 10 * time.Millisecond + + // And a Git repository status check that returns an error + origCheckGit := checkGitRepositoryStatus + origCheckKustom := checkKustomizationStatus + defer func() { + checkGitRepositoryStatus = origCheckGit + checkKustomizationStatus = origCheckKustom + }() + checkGitRepositoryStatus = func(string) error { return fmt.Errorf("git repo error") } + checkKustomizationStatus = func(string, []string) (map[string]bool, error) { + return map[string]bool{"k1": true}, nil + } + + // When waiting for kustomizations to be ready + err := handler.WaitForKustomizations() + + // Then a Git repository error should be returned + if err == nil || !strings.Contains(err.Error(), "git repository error") { + t.Errorf("expected git repository error, got %v", err) + } + }) + + t.Run("KustomizationStatusError", func(t *testing.T) { + // Given a blueprint handler with a kustomization + handler := &BaseBlueprintHandler{ + blueprint: blueprintv1alpha1.Blueprint{ + Kustomizations: []blueprintv1alpha1.Kustomization{ + {Name: "k1", Timeout: &metav1.Duration{Duration: 100 * time.Millisecond}}, + }, + }, + } + handler.kustomizationWaitPollInterval = 10 * time.Millisecond + + // And a kustomization status check that returns an error + origCheckGit := checkGitRepositoryStatus + origCheckKustom := checkKustomizationStatus + defer func() { + checkGitRepositoryStatus = origCheckGit + checkKustomizationStatus = origCheckKustom + }() + checkGitRepositoryStatus = func(string) error { return nil } + checkKustomizationStatus = func(string, []string) (map[string]bool, error) { + return nil, fmt.Errorf("kustomization error") + } + + // When waiting for kustomizations to be ready + err := handler.WaitForKustomizations() + + // Then a kustomization error should be returned + if err == nil || !strings.Contains(err.Error(), "kustomization error") { + t.Errorf("expected kustomization error, got %v", err) + } + }) +} diff --git a/pkg/blueprint/mock_blueprint_handler.go b/pkg/blueprint/mock_blueprint_handler.go index 869a2f2c8..7f960ab60 100644 --- a/pkg/blueprint/mock_blueprint_handler.go +++ b/pkg/blueprint/mock_blueprint_handler.go @@ -21,6 +21,7 @@ type MockBlueprintHandler struct { InstallFunc func() error GetRepositoryFunc func() blueprintv1alpha1.Repository SetRepositoryFunc func(repository blueprintv1alpha1.Repository) error + WaitForKustomizationsFunc func() error } // ============================================================================= @@ -152,5 +153,13 @@ func (m *MockBlueprintHandler) SetRepository(repository blueprintv1alpha1.Reposi return nil } +// WaitForKustomizations calls the mock WaitForKustomizationsFunc if set, otherwise returns nil +func (m *MockBlueprintHandler) WaitForKustomizations() error { + if m.WaitForKustomizationsFunc != nil { + return m.WaitForKustomizationsFunc() + } + return nil +} + // Ensure MockBlueprintHandler implements BlueprintHandler var _ BlueprintHandler = (*MockBlueprintHandler)(nil) diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index e4ac98718..7b6e70751 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -36,6 +36,11 @@ const ( DEFAULT_FLUX_KUSTOMIZATION_TIMEOUT = 5 * time.Minute DEFAULT_FLUX_SOURCE_INTERVAL = 1 * time.Minute DEFAULT_FLUX_SOURCE_TIMEOUT = 2 * time.Minute + + // Used for aggregate CLI wait (not per-resource) + DEFAULT_KUSTOMIZATION_WAIT_TOTAL_TIMEOUT = 10 * time.Minute + // Poll interval for CLI WaitForKustomizations + DEFAULT_KUSTOMIZATION_WAIT_POLL_INTERVAL = 5 * time.Second ) // Default AWS settings From 7d30a362b41f152a1f8ed88e6ba0e472eb247b35 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Fri, 16 May 2025 09:26:36 -0400 Subject: [PATCH 3/3] Revert to cascade merge --- api/v1alpha1/blueprint_types.go | 12 +- api/v1alpha1/blueprint_types_test.go | 165 ++++++++++++++++++++++++--- 2 files changed, 150 insertions(+), 27 deletions(-) diff --git a/api/v1alpha1/blueprint_types.go b/api/v1alpha1/blueprint_types.go index ca5c40731..3d96962b2 100644 --- a/api/v1alpha1/blueprint_types.go +++ b/api/v1alpha1/blueprint_types.go @@ -313,17 +313,13 @@ func (b *Blueprint) Merge(overlay *Blueprint) { // Merge the Reference type inline, prioritizing the first non-empty field if overlay.Repository.Ref.Commit != "" { b.Repository.Ref.Commit = overlay.Repository.Ref.Commit - } - if overlay.Repository.Ref.Name != "" { + } else if overlay.Repository.Ref.Name != "" { b.Repository.Ref.Name = overlay.Repository.Ref.Name - } - if overlay.Repository.Ref.SemVer != "" { + } else if overlay.Repository.Ref.SemVer != "" { b.Repository.Ref.SemVer = overlay.Repository.Ref.SemVer - } - if overlay.Repository.Ref.Tag != "" { + } else if overlay.Repository.Ref.Tag != "" { b.Repository.Ref.Tag = overlay.Repository.Ref.Tag - } - if overlay.Repository.Ref.Branch != "" { + } else if overlay.Repository.Ref.Branch != "" { b.Repository.Ref.Branch = overlay.Repository.Ref.Branch } diff --git a/api/v1alpha1/blueprint_types_test.go b/api/v1alpha1/blueprint_types_test.go index f23a37302..33b8aad17 100644 --- a/api/v1alpha1/blueprint_types_test.go +++ b/api/v1alpha1/blueprint_types_test.go @@ -442,7 +442,135 @@ func TestBlueprint_Merge(t *testing.T) { } }) - t.Run("OverlayRepositoryRefFields", func(t *testing.T) { + t.Run("OverlayRepositoryRefFirstNonEmptyField", func(t *testing.T) { + cases := []struct { + name string + ref Reference + check func(t *testing.T, ref Reference) + }{ + { + name: "Commit", + ref: Reference{Commit: "abc123", Name: "v1", SemVer: "1.0.0", Tag: "v1.0.0", Branch: "develop"}, + check: func(t *testing.T, ref Reference) { + if ref.Commit != "abc123" { + t.Errorf("Commit not set") + } + if ref.Name != "" { + t.Errorf("Name should be empty") + } + if ref.SemVer != "" { + t.Errorf("SemVer should be empty") + } + if ref.Tag != "" { + t.Errorf("Tag should be empty") + } + if ref.Branch != "main" { + t.Errorf("Branch should remain 'main'") + } + }, + }, + { + name: "Name", + ref: Reference{Name: "v1", SemVer: "1.0.0", Tag: "v1.0.0", Branch: "develop"}, + check: func(t *testing.T, ref Reference) { + if ref.Commit != "" { + t.Errorf("Commit should be empty") + } + if ref.Name != "v1" { + t.Errorf("Name not set") + } + if ref.SemVer != "" { + t.Errorf("SemVer should be empty") + } + if ref.Tag != "" { + t.Errorf("Tag should be empty") + } + if ref.Branch != "main" { + t.Errorf("Branch should remain 'main'") + } + }, + }, + { + name: "SemVer", + ref: Reference{SemVer: "1.0.0", Tag: "v1.0.0", Branch: "develop"}, + check: func(t *testing.T, ref Reference) { + if ref.Commit != "" { + t.Errorf("Commit should be empty") + } + if ref.Name != "" { + t.Errorf("Name should be empty") + } + if ref.SemVer != "1.0.0" { + t.Errorf("SemVer not set") + } + if ref.Tag != "" { + t.Errorf("Tag should be empty") + } + if ref.Branch != "main" { + t.Errorf("Branch should remain 'main'") + } + }, + }, + { + name: "Tag", + ref: Reference{Tag: "v1.0.0", Branch: "develop"}, + check: func(t *testing.T, ref Reference) { + if ref.Commit != "" { + t.Errorf("Commit should be empty") + } + if ref.Name != "" { + t.Errorf("Name should be empty") + } + if ref.SemVer != "" { + t.Errorf("SemVer should be empty") + } + if ref.Tag != "v1.0.0" { + t.Errorf("Tag not set") + } + if ref.Branch != "main" { + t.Errorf("Branch should remain 'main'") + } + }, + }, + { + name: "Branch", + ref: Reference{Branch: "develop"}, + check: func(t *testing.T, ref Reference) { + if ref.Commit != "" { + t.Errorf("Commit should be empty") + } + if ref.Name != "" { + t.Errorf("Name should be empty") + } + if ref.SemVer != "" { + t.Errorf("SemVer should be empty") + } + if ref.Tag != "" { + t.Errorf("Tag should be empty") + } + if ref.Branch != "develop" { + t.Errorf("Branch not set") + } + }, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + base := &Blueprint{ + Repository: Repository{ + Ref: Reference{Branch: "main"}, + }, + } + overlay := &Blueprint{ + Repository: Repository{Ref: tc.ref}, + } + base.Merge(overlay) + tc.check(t, base.Repository.Ref) + }) + } + }) + + t.Run("OverlayWithRepositoryRefFields", func(t *testing.T) { base := &Blueprint{ Repository: Repository{ Ref: Reference{ @@ -453,7 +581,6 @@ func TestBlueprint_Merge(t *testing.T) { overlay := &Blueprint{ Repository: Repository{ Ref: Reference{ - Commit: "abc123", Name: "v1", SemVer: "1.0.0", Tag: "v1.0.0", @@ -462,24 +589,24 @@ func TestBlueprint_Merge(t *testing.T) { }, } base.Merge(overlay) - if base.Repository.Ref.Commit != "abc123" { - t.Errorf("Expected Repository.Ref.Commit to be 'abc123', but got '%s'", base.Repository.Ref.Commit) + if base.Repository.Ref.Commit != "" { + t.Errorf("Expected Repository.Ref.Commit to be '', but got '%s'", base.Repository.Ref.Commit) } if base.Repository.Ref.Name != "v1" { t.Errorf("Expected Repository.Ref.Name to be 'v1', but got '%s'", base.Repository.Ref.Name) } - if base.Repository.Ref.SemVer != "1.0.0" { - t.Errorf("Expected Repository.Ref.SemVer to be '1.0.0', but got '%s'", base.Repository.Ref.SemVer) + if base.Repository.Ref.SemVer != "" { + t.Errorf("Expected Repository.Ref.SemVer to be '', but got '%s'", base.Repository.Ref.SemVer) } - if base.Repository.Ref.Tag != "v1.0.0" { - t.Errorf("Expected Repository.Ref.Tag to be 'v1.0.0', but got '%s'", base.Repository.Ref.Tag) + if base.Repository.Ref.Tag != "" { + t.Errorf("Expected Repository.Ref.Tag to be '', but got '%s'", base.Repository.Ref.Tag) } - if base.Repository.Ref.Branch != "develop" { - t.Errorf("Expected Repository.Ref.Branch to be 'develop', but got '%s'", base.Repository.Ref.Branch) + if base.Repository.Ref.Branch != "main" { + t.Errorf("Expected Repository.Ref.Branch to remain 'main', but got '%s'", base.Repository.Ref.Branch) } }) - t.Run("OverlayNonEmptyKustomizations", func(t *testing.T) { + t.Run("OverlayWithEmptyKustomizations", func(t *testing.T) { base := &Blueprint{ Kustomizations: []Kustomization{{Name: "A"}}, } @@ -656,17 +783,17 @@ func TestBlueprint_Merge(t *testing.T) { if base.Repository.Ref.Commit != "abc123" { t.Errorf("Expected Repository.Ref.Commit to be 'abc123', but got '%s'", base.Repository.Ref.Commit) } - if base.Repository.Ref.Name != "v1" { - t.Errorf("Expected Repository.Ref.Name to be 'v1', but got '%s'", base.Repository.Ref.Name) + if base.Repository.Ref.Name != "" { + t.Errorf("Expected Repository.Ref.Name to be '', but got '%s'", base.Repository.Ref.Name) } - if base.Repository.Ref.SemVer != "1.0.0" { - t.Errorf("Expected Repository.Ref.SemVer to be '1.0.0', but got '%s'", base.Repository.Ref.SemVer) + if base.Repository.Ref.SemVer != "" { + t.Errorf("Expected Repository.Ref.SemVer to be '', but got '%s'", base.Repository.Ref.SemVer) } - if base.Repository.Ref.Tag != "v1.0.0" { - t.Errorf("Expected Repository.Ref.Tag to be 'v1.0.0', but got '%s'", base.Repository.Ref.Tag) + if base.Repository.Ref.Tag != "" { + t.Errorf("Expected Repository.Ref.Tag to be '', but got '%s'", base.Repository.Ref.Tag) } - if base.Repository.Ref.Branch != "develop" { - t.Errorf("Expected Repository.Ref.Branch to be 'develop', but got '%s'", base.Repository.Ref.Branch) + if base.Repository.Ref.Branch != "main" { + t.Errorf("Expected Repository.Ref.Branch to remain 'main', but got '%s'", base.Repository.Ref.Branch) } })