From 08acbb5b2b5ad0151ad1ab046b558065a56f0cca Mon Sep 17 00:00:00 2001 From: Florian Bergmann Date: Mon, 11 May 2026 12:31:54 +0200 Subject: [PATCH 1/6] refactor: move promote business logic from cmd/promote/utils to pkg/promote Move all domain types (Service, Application, CodeComponent, AppInterfaceClone, ServicesRegistry, Repo) and business logic (Promote engine, PromoteCallbacks, target filtering, commit message formatting) from cmd/promote/utils/ to pkg/promote/. This follows the established project convention (pkg/controller/rotatesecret.go) of separating business logic from CLI wiring. The cmd/promote/ subcommands now import from pkg/promote instead of a sibling cmd/ package. Package renamed from 'utils' to 'promote' to give the domain its proper identity. No logic changes - pure move and import path update. All existing tests pass unchanged. --- cmd/promote/dynatrace/dt_utils.go | 6 +- cmd/promote/dynatrace/dynatrace.go | 8 +-- cmd/promote/managedscripts/managed_scripts.go | 12 ++-- .../managedscripts/managed_scripts_test.go | 30 ++++---- cmd/promote/saas/saas.go | 26 +++---- cmd/promote/saas/saas_test.go | 68 +++++++++---------- .../promote}/app_interface_clone.go | 2 +- .../promote}/app_interface_clone_test.go | 2 +- .../promote/utils => pkg/promote}/git_repo.go | 2 +- {cmd/promote/utils => pkg/promote}/service.go | 2 +- .../utils => pkg/promote}/service_test.go | 2 +- .../promote}/services_registry.go | 2 +- .../promote}/services_registry_test.go | 2 +- .../utils => pkg/promote}/test_tools.go | 2 +- .../utils => pkg/promote}/utils_test.go | 2 +- 15 files changed, 84 insertions(+), 84 deletions(-) rename {cmd/promote/utils => pkg/promote}/app_interface_clone.go (99%) rename {cmd/promote/utils => pkg/promote}/app_interface_clone_test.go (99%) rename {cmd/promote/utils => pkg/promote}/git_repo.go (99%) rename {cmd/promote/utils => pkg/promote}/service.go (99%) rename {cmd/promote/utils => pkg/promote}/service_test.go (99%) rename {cmd/promote/utils => pkg/promote}/services_registry.go (99%) rename {cmd/promote/utils => pkg/promote}/services_registry_test.go (99%) rename {cmd/promote/utils => pkg/promote}/test_tools.go (99%) rename {cmd/promote/utils => pkg/promote}/utils_test.go (91%) diff --git a/cmd/promote/dynatrace/dt_utils.go b/cmd/promote/dynatrace/dt_utils.go index 6449ffc83..527643a55 100644 --- a/cmd/promote/dynatrace/dt_utils.go +++ b/cmd/promote/dynatrace/dt_utils.go @@ -10,7 +10,7 @@ import ( "strings" "github.com/openshift/osdctl/cmd/promote/iexec" - "github.com/openshift/osdctl/cmd/promote/utils" + "github.com/openshift/osdctl/pkg/promote" kyaml "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -35,7 +35,7 @@ func validateDynatraceServiceFilePath(filePath string) string { return filePath } -func getResourceTemplatesPaths(serviceRegistry *utils.ServicesRegistry, serviceId string) string { +func getResourceTemplatesPaths(serviceRegistry *promote.ServicesRegistry, serviceId string) string { service, err := serviceRegistry.GetService(serviceId) if err != nil { return "" @@ -58,7 +58,7 @@ func getResourceTemplatesPaths(serviceRegistry *utils.ServicesRegistry, serviceI return strings.Join(paths, ", ") } -func listServiceIds(serviceRegistry *utils.ServicesRegistry) error { +func listServiceIds(serviceRegistry *promote.ServicesRegistry) error { serviceIds := serviceRegistry.GetServicesIds() fmt.Println("### Available Dynatrace components ###") diff --git a/cmd/promote/dynatrace/dynatrace.go b/cmd/promote/dynatrace/dynatrace.go index 795eeefe7..0b3f4ed4c 100644 --- a/cmd/promote/dynatrace/dynatrace.go +++ b/cmd/promote/dynatrace/dynatrace.go @@ -4,7 +4,7 @@ import ( "errors" "fmt" - "github.com/openshift/osdctl/cmd/promote/utils" + "github.com/openshift/osdctl/pkg/promote" "github.com/spf13/cobra" ) @@ -89,12 +89,12 @@ TERRAFORM MODULES: } else { ops.validateSaasFlow() - appInterfaceClone, err := utils.FindAppInterfaceClone(ops.appInterfaceProvidedPath) + appInterfaceClone, err := promote.FindAppInterfaceClone(ops.appInterfaceProvidedPath) if err != nil { return err } - servicesRegistry, err := utils.NewServicesRegistry( + servicesRegistry, err := promote.NewServicesRegistry( appInterfaceClone, validateDynatraceServiceFilePath, saasDynatraceDir, @@ -122,7 +122,7 @@ TERRAFORM MODULES: if err != nil { return err } - err = service.Promote(&utils.DefaultPromoteCallbacks{Service: service}, ops.gitHash) + err = service.Promote(&promote.DefaultPromoteCallbacks{Service: service}, ops.gitHash) if err != nil { return fmt.Errorf("error while promoting service: %v", err) diff --git a/cmd/promote/managedscripts/managed_scripts.go b/cmd/promote/managedscripts/managed_scripts.go index 60d180e1d..01d552e74 100644 --- a/cmd/promote/managedscripts/managed_scripts.go +++ b/cmd/promote/managedscripts/managed_scripts.go @@ -4,7 +4,7 @@ import ( "fmt" "path/filepath" - "github.com/openshift/osdctl/cmd/promote/utils" + "github.com/openshift/osdctl/pkg/promote" "github.com/spf13/cobra" kyaml "sigs.k8s.io/kustomize/kyaml/yaml" @@ -23,11 +23,11 @@ type managedScriptsOptions struct { } type promoteCallbacks struct { - utils.DefaultPromoteCallbacks + promote.DefaultPromoteCallbacks } func (c *promoteCallbacks) FilterTargets(targetNodes []*kyaml.RNode) ([]*kyaml.RNode, error) { - return utils.FilterTargetsContainingNamespaceRef(targetNodes, prodNamespaceRef) + return promote.FilterTargetsContainingNamespaceRef(targetNodes, prodNamespaceRef) } func (*promoteCallbacks) GetResourceTemplateRepoUrl(*kyaml.RNode) (string, error) { @@ -68,14 +68,14 @@ func NewCmdManagedScripts() *cobra.Command { # Promote managed-scripts repo osdctl promote managedscripts --gitHash `, RunE: func(cmd *cobra.Command, args []string) error { - appInterfaceClone, err := utils.FindAppInterfaceClone(ops.appInterfaceProvidedPath) + appInterfaceClone, err := promote.FindAppInterfaceClone(ops.appInterfaceProvidedPath) if err != nil { return err } cmd.SilenceUsage = true - service, err := utils.ReadServiceFromFile( + service, err := promote.ReadServiceFromFile( appInterfaceClone, filepath.Join(appInterfaceClone.GetPath(), serviceRelPath)) if err != nil { @@ -83,7 +83,7 @@ func NewCmdManagedScripts() *cobra.Command { } return service.Promote(&promoteCallbacks{ - DefaultPromoteCallbacks: utils.DefaultPromoteCallbacks{Service: service}, + DefaultPromoteCallbacks: promote.DefaultPromoteCallbacks{Service: service}, }, ops.gitHash) }, } diff --git a/cmd/promote/managedscripts/managed_scripts_test.go b/cmd/promote/managedscripts/managed_scripts_test.go index 336106f3c..c50a363a3 100644 --- a/cmd/promote/managedscripts/managed_scripts_test.go +++ b/cmd/promote/managedscripts/managed_scripts_test.go @@ -8,7 +8,7 @@ import ( "testing" "github.com/go-git/go-git/v5" - "github.com/openshift/osdctl/cmd/promote/utils" + "github.com/openshift/osdctl/pkg/promote" kyaml "sigs.k8s.io/kustomize/kyaml/yaml" . "github.com/onsi/ginkgo" @@ -44,13 +44,13 @@ func TestSetup(t *testing.T) { } type managedScriptsTestData struct { - *utils.TestData + *promote.TestData managedScriptsRepoPath string managedScriptsRepoHashes [10]string } -func CreateManagedScriptsTestData(nestedData *utils.TestData) *managedScriptsTestData { +func CreateManagedScriptsTestData(nestedData *promote.TestData) *managedScriptsTestData { data := managedScriptsTestData{ TestData: nestedData, @@ -83,7 +83,7 @@ func CreateManagedScriptsTestData(nestedData *utils.TestData) *managedScriptsTes } hash, err := managedscriptsWorkTree.Commit(fmt.Sprintf("Commit #%d", k), &git.CommitOptions{ - Author: &utils.DefaultSignature, + Author: &promote.DefaultSignature, AllowEmptyCommits: true, }) Expect(err).ShouldNot(HaveOccurred()) @@ -98,7 +98,7 @@ func (d *managedScriptsTestData) GetManagedScriptsRepoFormattedLog(hashIndexes . var sb strings.Builder for _, idx := range hashIndexes { - fmt.Fprintf(&sb, utils.CommitTemplate, d.managedScriptsRepoHashes[idx], idx) + fmt.Fprintf(&sb, promote.CommitTemplate, d.managedScriptsRepoHashes[idx], idx) } return sb.String() @@ -116,30 +116,30 @@ func (c *promoteCallbacksMock) GetResourceTemplateRepoUrl(*kyaml.RNode) (string, var _ = Describe("Service struct", func() { var data *managedScriptsTestData - var service *utils.Service + var service *promote.Service BeforeEach(func() { var properties map[string]string - data = CreateManagedScriptsTestData(utils.CreateTestData(func(data *utils.TestData) map[string]string { - properties = utils.InitProperties(data.TestRepoPath, data.TestRepoHashes[1]) + data = CreateManagedScriptsTestData(promote.CreateTestData(func(data *promote.TestData) map[string]string { + properties = promote.InitProperties(data.TestRepoPath, data.TestRepoHashes[1]) return map[string]string{ - "data/services/backplane/app.yaml": utils.GetFileContent(utils.AppFileContentTemplate, "backplane", properties), + "data/services/backplane/app.yaml": promote.GetFileContent(promote.AppFileContentTemplate, "backplane", properties), } })) properties["managedScriptsGitHash"] = data.managedScriptsRepoHashes[2] - data.WriteAppInterfaceFile(serviceRelPath, utils.GetFileContent(serviceFileContentBackplaneTemplate, "", properties)) + data.WriteAppInterfaceFile(serviceRelPath, promote.GetFileContent(serviceFileContentBackplaneTemplate, "", properties)) data.CommitAppInterfaceChanges("Defining the service to promote") }) JustBeforeEach(func() { var err error - appInterfaceClone, err := utils.FindAppInterfaceClone(data.AppInterfacePath) + appInterfaceClone, err := promote.FindAppInterfaceClone(data.AppInterfacePath) Expect(err).ShouldNot(HaveOccurred()) - service, err = utils.ReadServiceFromFile( + service, err = promote.ReadServiceFromFile( appInterfaceClone, filepath.Join(appInterfaceClone.GetPath(), serviceRelPath)) Expect(err).ShouldNot(HaveOccurred()) @@ -147,19 +147,19 @@ var _ = Describe("Service struct", func() { }) AfterEach(func() { - utils.CleanupAllTestDataResources() + promote.CleanupAllTestDataResources() }) Context("Promote method", func() { When("namespaceRef is set to 'hivep'", func() { It("promotes all targets in all resource templates", func() { // because all namespaces have their ref contain that string err := service.Promote(&promoteCallbacksMock{ - promoteCallbacks: promoteCallbacks{DefaultPromoteCallbacks: utils.DefaultPromoteCallbacks{Service: service}}, + promoteCallbacks: promoteCallbacks{DefaultPromoteCallbacks: promote.DefaultPromoteCallbacks{Service: service}}, data: data, }, data.managedScriptsRepoHashes[8]) Expect(err).ShouldNot(HaveOccurred()) - expectedProperties := utils.InitProperties(data.TestRepoPath, data.TestRepoHashes[1]) + expectedProperties := promote.InitProperties(data.TestRepoPath, data.TestRepoHashes[1]) expectedProperties["managedScriptsGitHash"] = data.managedScriptsRepoHashes[8] data.CheckAppInterfaceFileContent(serviceRelPath, serviceFileContentBackplaneTemplate, "", expectedProperties) diff --git a/cmd/promote/saas/saas.go b/cmd/promote/saas/saas.go index 520491c3c..a0da31dbe 100644 --- a/cmd/promote/saas/saas.go +++ b/cmd/promote/saas/saas.go @@ -7,7 +7,7 @@ import ( "path/filepath" "strings" - "github.com/openshift/osdctl/cmd/promote/utils" + "github.com/openshift/osdctl/pkg/promote" "github.com/spf13/cobra" kyaml "sigs.k8s.io/kustomize/kyaml/yaml" @@ -45,11 +45,11 @@ func validateSaasServiceFilePath(filePath string) string { } type promoteCallbacks struct { - utils.DefaultPromoteCallbacks + promote.DefaultPromoteCallbacks namespaceRef string isHotfix bool - component *utils.CodeComponent // not supposed to change on subsequent calls to ComputeCommitMessage + component *promote.CodeComponent // not supposed to change on subsequent calls to ComputeCommitMessage } func (c *promoteCallbacks) FilterTargets(targetNodes []*kyaml.RNode) ([]*kyaml.RNode, error) { @@ -88,11 +88,11 @@ func (c *promoteCallbacks) FilterTargets(targetNodes []*kyaml.RNode) ([]*kyaml.R } } - namespaceRef = utils.DefaultProdNamespaceRef + namespaceRef = promote.DefaultProdNamespaceRef } } - return utils.FilterTargetsContainingNamespaceRef(targetNodes, namespaceRef) + return promote.FilterTargetsContainingNamespaceRef(targetNodes, namespaceRef) } // readE2EServiceName reads the e2e test service file to find the actual @@ -105,7 +105,7 @@ func (c *promoteCallbacks) FilterTargets(targetNodes []*kyaml.RNode) ([]*kyaml.R // E2E service name: saas-configure-am-operator-e2e-test (abbreviated!) // // This function handles the inconsistency by reading the actual YAML file. -func readE2EServiceName(service *utils.Service) (string, error) { +func readE2EServiceName(service *promote.Service) (string, error) { serviceFilePath := service.GetFilePath() serviceDirPath := "" if filepath.Base(serviceFilePath) == "deploy.yaml" { @@ -116,7 +116,7 @@ func readE2EServiceName(service *utils.Service) (string, error) { e2eTestPath := filepath.Join(serviceDirPath, "osde2e-focus-test.yaml") - e2eService, err := utils.ReadYamlDocFromFile(e2eTestPath) + e2eService, err := promote.ReadYamlDocFromFile(e2eTestPath) if err != nil { return "", fmt.Errorf("failed to read e2e test file: %w", err) } @@ -124,7 +124,7 @@ func readE2EServiceName(service *utils.Service) (string, error) { return e2eService.GetName(), nil } -func computeE2EServiceName(service *utils.Service, componentName string) string { +func computeE2EServiceName(service *promote.Service, componentName string) string { // Try to discover the actual e2e test service name from app-interface e2eServiceName, err := readE2EServiceName(service) if err != nil { @@ -149,7 +149,7 @@ func computeE2EServiceName(service *utils.Service, componentName string) string // - 7-day time window // // If discovery fails, falls back to standard naming convention. -func generateTestLogsURL(service *utils.Service, componentName, e2eServiceName, gitHash, env string) string { +func generateTestLogsURL(service *promote.Service, componentName, e2eServiceName, gitHash, env string) string { if env == "" { env = "osd-stage-hives02ue1" } @@ -176,7 +176,7 @@ func generateTestLogsURL(service *utils.Service, componentName, e2eServiceName, return url } -func (c *promoteCallbacks) ComputeCommitMessage(resourceTemplateRepo *utils.Repo, resourceTemplatePath, currentHash, newHash string) (*utils.CommitMessage, error) { +func (c *promoteCallbacks) ComputeCommitMessage(resourceTemplateRepo *promote.Repo, resourceTemplatePath, currentHash, newHash string) (*promote.CommitMessage, error) { commitMessage, err := c.DefaultPromoteCallbacks.ComputeCommitMessage(resourceTemplateRepo, resourceTemplatePath, currentHash, newHash) if err != nil { return nil, err @@ -234,12 +234,12 @@ func NewCmdSaas() *cobra.Command { # Promote a SaaS service/operator osdctl promote saas --serviceId --gitHash `, RunE: func(cmd *cobra.Command, args []string) error { - appInterfaceClone, err := utils.FindAppInterfaceClone(ops.appInterfaceProvidedPath) + appInterfaceClone, err := promote.FindAppInterfaceClone(ops.appInterfaceProvidedPath) if err != nil { return err } - servicesRegistry, err := utils.NewServicesRegistry( + servicesRegistry, err := promote.NewServicesRegistry( appInterfaceClone, validateSaasServiceFilePath, osdSaasDirPath, BpSaasDirPath, cadSaasDirPath, @@ -276,7 +276,7 @@ func NewCmdSaas() *cobra.Command { } return service.Promote(&promoteCallbacks{ - DefaultPromoteCallbacks: utils.DefaultPromoteCallbacks{Service: service}, + DefaultPromoteCallbacks: promote.DefaultPromoteCallbacks{Service: service}, namespaceRef: ops.namespaceRef, isHotfix: ops.isHotfix, }, ops.gitHash) diff --git a/cmd/promote/saas/saas_test.go b/cmd/promote/saas/saas_test.go index 5348bad38..709b2a949 100644 --- a/cmd/promote/saas/saas_test.go +++ b/cmd/promote/saas/saas_test.go @@ -5,13 +5,13 @@ import ( "strings" "testing" - "github.com/openshift/osdctl/cmd/promote/utils" + "github.com/openshift/osdctl/pkg/promote" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) -var serviceFileContentCanaryTemplate = strings.Replace(utils.ServiceFileContentTemplate, +var serviceFileContentCanaryTemplate = strings.Replace(promote.ServiceFileContentTemplate, "name: hivep01", "name: hivep01"+defaultProdTargetNameSuffix, 1) @@ -22,29 +22,29 @@ func TestSetup(t *testing.T) { } var _ = Describe("ServicesRegistry struct", func() { - var data *utils.TestData - var servicesRegistry *utils.ServicesRegistry + var data *promote.TestData + var servicesRegistry *promote.ServicesRegistry BeforeEach(func() { - data = utils.CreateTestData(func(data *utils.TestData) map[string]string { - properties := utils.InitProperties(data.TestRepoPath, data.TestRepoHashes[0]) + data = promote.CreateTestData(func(data *promote.TestData) map[string]string { + properties := promote.InitProperties(data.TestRepoPath, data.TestRepoHashes[0]) return map[string]string{ - "data/services/gen-app/cicd/saas/saas-service-1.yaml": utils.GetFileContent(utils.ServiceFileContentTemplate, "service-1", properties), - "data/services/gen-app/cicd/saas/service-2.yaml": utils.GetFileContent(utils.ServiceFileContentTemplate, "service-2", properties), - "data/services/other-app/cicd/saas/saas-service-3/deploy.yaml": utils.GetFileContent(utils.ServiceFileContentTemplate, "service-3", properties), - "data/services/gen-app/cicd/saas/service-4/deploy.yaml": utils.GetFileContent(utils.ServiceFileContentTemplate, "service-4", properties), + "data/services/gen-app/cicd/saas/saas-service-1.yaml": promote.GetFileContent(promote.ServiceFileContentTemplate, "service-1", properties), + "data/services/gen-app/cicd/saas/service-2.yaml": promote.GetFileContent(promote.ServiceFileContentTemplate, "service-2", properties), + "data/services/other-app/cicd/saas/saas-service-3/deploy.yaml": promote.GetFileContent(promote.ServiceFileContentTemplate, "service-3", properties), + "data/services/gen-app/cicd/saas/service-4/deploy.yaml": promote.GetFileContent(promote.ServiceFileContentTemplate, "service-4", properties), - "data/services/gen-app/app.yml": utils.GetFileContent(utils.AppFileContentTemplate, "gen-app", properties), + "data/services/gen-app/app.yml": promote.GetFileContent(promote.AppFileContentTemplate, "gen-app", properties), } }) - servicesRegistry = utils.CreateServiceRegistry(data, + servicesRegistry = promote.CreateServiceRegistry(data, validateSaasServiceFilePath, "data/services/gen-app/cicd/saas", "data/services/other-app/cicd/saas") }) AfterEach(func() { - utils.CleanupAllTestDataResources() + promote.CleanupAllTestDataResources() }) When("querying the registry", func() { @@ -64,16 +64,16 @@ var _ = Describe("ServicesRegistry struct", func() { }) var _ = Describe("Service struct", func() { - var data *utils.TestData - var service *utils.Service + var data *promote.TestData + var service *promote.Service BeforeEach(func() { - data = utils.CreateTestData(func(data *utils.TestData) map[string]string { - properties := utils.InitProperties(data.TestRepoPath, data.TestRepoHashes[0]) + data = promote.CreateTestData(func(data *promote.TestData) map[string]string { + properties := promote.InitProperties(data.TestRepoPath, data.TestRepoHashes[0]) return map[string]string{ - "data/services/gen-app/cicd/saas/service-1.yaml": utils.GetFileContent(serviceFileContentCanaryTemplate, "service-1", properties), - "data/services/gen-app/app.yml": utils.GetFileContent(utils.AppFileContentTemplate, "gen-app", properties), + "data/services/gen-app/cicd/saas/service-1.yaml": promote.GetFileContent(serviceFileContentCanaryTemplate, "service-1", properties), + "data/services/gen-app/app.yml": promote.GetFileContent(promote.AppFileContentTemplate, "gen-app", properties), } }) }) @@ -81,22 +81,22 @@ var _ = Describe("Service struct", func() { JustBeforeEach(func() { var err error - servicesRegistry := utils.CreateDefaultServiceRegistry(data) + servicesRegistry := promote.CreateDefaultServiceRegistry(data) service, err = servicesRegistry.GetService("service-1") Expect(err).ShouldNot(HaveOccurred()) Expect(service).ToNot(BeNil()) }) AfterEach(func() { - utils.CleanupAllTestDataResources() + promote.CleanupAllTestDataResources() }) Context("Promote method", func() { When("namespaceRef is set to 'hivep'", func() { It("promotes all targets in all resource templates", func() { // because all namespaces have their ref contain that string err := service.Promote(&promoteCallbacks{ - DefaultPromoteCallbacks: utils.DefaultPromoteCallbacks{Service: service}, - namespaceRef: utils.DefaultProdNamespaceRef, + DefaultPromoteCallbacks: promote.DefaultPromoteCallbacks{Service: service}, + namespaceRef: promote.DefaultProdNamespaceRef, isHotfix: false, }, data.TestRepoHashes[5]) Expect(err).ShouldNot(HaveOccurred()) @@ -114,8 +114,8 @@ var _ = Describe("Service struct", func() { It("still promotes all targets in all resource templates but the links in the commit message change a bit", func() { err := service.Promote(&promoteCallbacks{ - DefaultPromoteCallbacks: utils.DefaultPromoteCallbacks{Service: service}, - namespaceRef: utils.DefaultProdNamespaceRef, + DefaultPromoteCallbacks: promote.DefaultPromoteCallbacks{Service: service}, + namespaceRef: promote.DefaultProdNamespaceRef, isHotfix: false, }, data.TestRepoHashes[5]) Expect(err).ShouldNot(HaveOccurred()) @@ -127,7 +127,7 @@ var _ = Describe("Service struct", func() { }) AfterEach(func() { - data.CheckAppInterfaceService1Content(serviceFileContentCanaryTemplate, utils.InitProperties(data.TestRepoPath, data.TestRepoHashes[5])) + data.CheckAppInterfaceService1Content(serviceFileContentCanaryTemplate, promote.InitProperties(data.TestRepoPath, data.TestRepoHashes[5])) data.CheckAppInterfaceIsClean() data.CheckAppInterfaceBranchName(fmt.Sprintf("promote-service-1-%s", data.TestRepoHashes[5])) @@ -145,13 +145,13 @@ var _ = Describe("Service struct", func() { When("namespaceRef is set to 'hivep02'", func() { It("only promotes those hivep02 targets", func() { err := service.Promote(&promoteCallbacks{ - DefaultPromoteCallbacks: utils.DefaultPromoteCallbacks{Service: service}, + DefaultPromoteCallbacks: promote.DefaultPromoteCallbacks{Service: service}, namespaceRef: "hivep02", isHotfix: false, }, data.TestRepoHashes[8]) Expect(err).ShouldNot(HaveOccurred()) - expectedProperties := utils.InitProperties(data.TestRepoPath, data.TestRepoHashes[0]) + expectedProperties := promote.InitProperties(data.TestRepoPath, data.TestRepoHashes[0]) expectedProperties["gitHashProd1Target2"] = data.TestRepoHashes[8] expectedProperties["gitHashProd2Target2"] = data.TestRepoHashes[8] data.CheckAppInterfaceService1Content(serviceFileContentCanaryTemplate, expectedProperties) @@ -172,13 +172,13 @@ var _ = Describe("Service struct", func() { When("namespaceRef is empty", func() { It("only promotes the canary target", func() { err := service.Promote(&promoteCallbacks{ - DefaultPromoteCallbacks: utils.DefaultPromoteCallbacks{Service: service}, + DefaultPromoteCallbacks: promote.DefaultPromoteCallbacks{Service: service}, namespaceRef: "", // empty namespaceRef means only considering the canary target isHotfix: false, }, data.TestRepoHashes[9]) Expect(err).ShouldNot(HaveOccurred()) - expectedProperties := utils.InitProperties(data.TestRepoPath, data.TestRepoHashes[0]) + expectedProperties := promote.InitProperties(data.TestRepoPath, data.TestRepoHashes[0]) expectedProperties["gitHashProd1Target1"] = data.TestRepoHashes[9] data.CheckAppInterfaceService1Content(serviceFileContentCanaryTemplate, expectedProperties) @@ -195,17 +195,17 @@ var _ = Describe("Service struct", func() { When("there is a hotfix", func() { It("promotes all targets in all resource templates & update the application file", func() { err := service.Promote(&promoteCallbacks{ - DefaultPromoteCallbacks: utils.DefaultPromoteCallbacks{Service: service}, + DefaultPromoteCallbacks: promote.DefaultPromoteCallbacks{Service: service}, namespaceRef: "", // empty namespaceRef normally means that only the canary target is considered, but in case of hotfix, this default to "hivep" isHotfix: true, }, data.TestRepoHashes[9]) Expect(err).ShouldNot(HaveOccurred()) - data.CheckAppInterfaceService1Content(serviceFileContentCanaryTemplate, utils.InitProperties(data.TestRepoPath, data.TestRepoHashes[9])) + data.CheckAppInterfaceService1Content(serviceFileContentCanaryTemplate, promote.InitProperties(data.TestRepoPath, data.TestRepoHashes[9])) - expectedAppProperties := utils.InitProperties(data.TestRepoPath, "") + expectedAppProperties := promote.InitProperties(data.TestRepoPath, "") expectedAppProperties["hotfixVersion"] = data.TestRepoHashes[9] - data.CheckAppInterfaceFileContent("data/services/gen-app/app.yml", utils.AppFileContentTemplateWithHotfixVersion, "gen-app", expectedAppProperties) + data.CheckAppInterfaceFileContent("data/services/gen-app/app.yml", promote.AppFileContentTemplateWithHotfixVersion, "gen-app", expectedAppProperties) data.CheckAppInterfaceIsClean() data.CheckAppInterfaceBranchName(fmt.Sprintf("promote-service-1-%s", data.TestRepoHashes[9])) diff --git a/cmd/promote/utils/app_interface_clone.go b/pkg/promote/app_interface_clone.go similarity index 99% rename from cmd/promote/utils/app_interface_clone.go rename to pkg/promote/app_interface_clone.go index 900492be3..cf3423b19 100644 --- a/cmd/promote/utils/app_interface_clone.go +++ b/pkg/promote/app_interface_clone.go @@ -1,4 +1,4 @@ -package utils +package promote import ( "fmt" diff --git a/cmd/promote/utils/app_interface_clone_test.go b/pkg/promote/app_interface_clone_test.go similarity index 99% rename from cmd/promote/utils/app_interface_clone_test.go rename to pkg/promote/app_interface_clone_test.go index d46081a62..04583e867 100644 --- a/cmd/promote/utils/app_interface_clone_test.go +++ b/pkg/promote/app_interface_clone_test.go @@ -1,4 +1,4 @@ -package utils +package promote import ( "os" diff --git a/cmd/promote/utils/git_repo.go b/pkg/promote/git_repo.go similarity index 99% rename from cmd/promote/utils/git_repo.go rename to pkg/promote/git_repo.go index 5437a1a17..c2af01624 100644 --- a/cmd/promote/utils/git_repo.go +++ b/pkg/promote/git_repo.go @@ -1,4 +1,4 @@ -package utils +package promote import ( "fmt" diff --git a/cmd/promote/utils/service.go b/pkg/promote/service.go similarity index 99% rename from cmd/promote/utils/service.go rename to pkg/promote/service.go index 6db190e53..9f9ab6551 100644 --- a/cmd/promote/utils/service.go +++ b/pkg/promote/service.go @@ -1,4 +1,4 @@ -package utils +package promote import ( "fmt" diff --git a/cmd/promote/utils/service_test.go b/pkg/promote/service_test.go similarity index 99% rename from cmd/promote/utils/service_test.go rename to pkg/promote/service_test.go index cbedae521..dbfb72fd0 100644 --- a/cmd/promote/utils/service_test.go +++ b/pkg/promote/service_test.go @@ -1,4 +1,4 @@ -package utils +package promote import ( "fmt" diff --git a/cmd/promote/utils/services_registry.go b/pkg/promote/services_registry.go similarity index 99% rename from cmd/promote/utils/services_registry.go rename to pkg/promote/services_registry.go index 9e453b87f..c068eafd0 100644 --- a/cmd/promote/utils/services_registry.go +++ b/pkg/promote/services_registry.go @@ -1,4 +1,4 @@ -package utils +package promote import ( "fmt" diff --git a/cmd/promote/utils/services_registry_test.go b/pkg/promote/services_registry_test.go similarity index 99% rename from cmd/promote/utils/services_registry_test.go rename to pkg/promote/services_registry_test.go index 9c42cdfc1..a95eafe88 100644 --- a/cmd/promote/utils/services_registry_test.go +++ b/pkg/promote/services_registry_test.go @@ -1,4 +1,4 @@ -package utils +package promote import ( "os" diff --git a/cmd/promote/utils/test_tools.go b/pkg/promote/test_tools.go similarity index 99% rename from cmd/promote/utils/test_tools.go rename to pkg/promote/test_tools.go index b7ab94649..9ba3810d8 100644 --- a/cmd/promote/utils/test_tools.go +++ b/pkg/promote/test_tools.go @@ -1,4 +1,4 @@ -package utils +package promote import ( "fmt" diff --git a/cmd/promote/utils/utils_test.go b/pkg/promote/utils_test.go similarity index 91% rename from cmd/promote/utils/utils_test.go rename to pkg/promote/utils_test.go index 85162c380..9d0044fcb 100644 --- a/cmd/promote/utils/utils_test.go +++ b/pkg/promote/utils_test.go @@ -1,4 +1,4 @@ -package utils +package promote import ( "testing" From d05d8829e652a3a4676ddb1c0c55675ff33f3253 Mon Sep 17 00:00:00 2001 From: Florian Bergmann Date: Mon, 11 May 2026 12:35:38 +0200 Subject: [PATCH 2/6] feat: add blockedVersions support for code components Add the ability to block specific SHA versions from being promoted through progressive delivery by adding them to codeComponents[].blockedVersions in the application's app.yaml file. New domain methods in pkg/promote: - Application.GetComponentByName(name) - find a component by name (vs URL) - CodeComponent.AddBlockedVersion(hash) - append to blockedVersions with deduplication; creates the field if absent, errors on duplicates New CLI command: osdctl promote blocked --serviceId --component --gitHash The command locates the app.yaml through the SaaS service file, finds the named component, appends the hash to blockedVersions, then branches and commits in app-interface. Includes 9 new unit tests covering: - GetComponentByName: found, not found, multiple components - AddBlockedVersion: create new field, append to existing, reject duplicates, append to multi-entry list --- cmd/promote/blocked/blocked.go | 146 ++++++++++++++++++++++++++++++ cmd/promote/cmd.go | 2 + pkg/promote/service.go | 61 +++++++++++++ pkg/promote/service_test.go | 159 +++++++++++++++++++++++++++++++++ pkg/promote/test_tools.go | 7 ++ 5 files changed, 375 insertions(+) create mode 100644 cmd/promote/blocked/blocked.go diff --git a/cmd/promote/blocked/blocked.go b/cmd/promote/blocked/blocked.go new file mode 100644 index 000000000..2771d3d95 --- /dev/null +++ b/cmd/promote/blocked/blocked.go @@ -0,0 +1,146 @@ +package blocked + +import ( + "fmt" + "path/filepath" + "strings" + + "github.com/openshift/osdctl/pkg/promote" + "github.com/spf13/cobra" +) + +type blockedOptions struct { + appInterfaceProvidedPath string + serviceId string + componentName string + gitHash string +} + +// NewCmdBlocked implements the blocked command to add a blocked version to a component in app.yaml +func NewCmdBlocked() *cobra.Command { + ops := &blockedOptions{} + blockedCmd := &cobra.Command{ + Use: "blocked", + Short: "Add a blocked version to a component in app.yaml", + Long: `Add a SHA commit hash to the blockedVersions list for a code component +in the application's app.yaml file. This prevents the specified version +from being promoted through progressive delivery. + +The command locates the app.yaml through the SaaS service file, finds +the specified component by name, and appends the git hash to its +codeComponents[].blockedVersions array. If the blockedVersions field +does not yet exist, it will be created. + +Duplicate entries are rejected with an error.`, + Args: cobra.NoArgs, + DisableAutoGenTag: true, + Example: ` + # Block a specific version for a component + osdctl promote blocked --serviceId --component --gitHash + + # With explicit app-interface path + osdctl promote blocked --serviceId --component --gitHash --appInterfaceDir /path/to/app-interface`, + RunE: func(cmd *cobra.Command, args []string) error { + if ops.serviceId == "" { + return fmt.Errorf("--serviceId is required") + } + if ops.componentName == "" { + return fmt.Errorf("--component is required") + } + if ops.gitHash == "" { + return fmt.Errorf("--gitHash is required") + } + + cmd.SilenceUsage = true + + appInterfaceClone, err := promote.FindAppInterfaceClone(ops.appInterfaceProvidedPath) + if err != nil { + return err + } + + isClean, err := appInterfaceClone.IsClean() + if err != nil { + return err + } + if !isClean { + return fmt.Errorf("app-interface clone in '%s' has uncommitted changes, please commit or stash them before proceeding", appInterfaceClone.GetPath()) + } + + servicesRegistry, err := promote.NewServicesRegistry( + appInterfaceClone, + func(filePath string) string { return filePath }, + "data/services/osd-operators/cicd/saas", + "data/services/backplane/cicd/saas", + "data/services/configuration-anomaly-detection/cicd", + ) + if err != nil { + return err + } + + service, err := servicesRegistry.GetService(ops.serviceId) + if err != nil { + return err + } + + application := service.GetApplication() + + component, err := application.GetComponentByName(ops.componentName) + if err != nil { + return err + } + + err = component.AddBlockedVersion(ops.gitHash) + if err != nil { + return err + } + + err = application.Save() + if err != nil { + return fmt.Errorf("failed to save application '%s': %v", application.GetFilePath(), err) + } + + branchName := fmt.Sprintf("block-%s-%s", ops.componentName, ops.gitHash) + err = appInterfaceClone.CheckoutNewBranch(branchName) + if err != nil { + return err + } + + commitMessage := fmt.Sprintf("Block version %s for %s\n\nAdd %s to blockedVersions for component '%s' in '%s'.", + ops.gitHash, + ops.componentName, + ops.gitHash, + ops.componentName, + filepath.Base(application.GetFilePath()), + ) + + err = appInterfaceClone.Commit(commitMessage) + if err != nil { + return err + } + + fmt.Println("SUCCESS!") + fmt.Printf("Blocked version %s for component %s\n", ops.gitHash, ops.componentName) + fmt.Printf("Application file: %s\n", application.GetFilePath()) + fmt.Println("") + fmt.Println("------------- Commit message -------------") + fmt.Println(commitMessage) + fmt.Println("------------- End of commit message -------------") + fmt.Println("") + fmt.Printf("Push the following branch on your fork and create a MR from it: %s\n", branchName) + + appInterfacePath := appInterfaceClone.GetPath() + if strings.Contains(appInterfacePath, "app-interface") { + fmt.Printf("\n(reminder: the push has to be run from the following Git clone: %s)\n", appInterfacePath) + } + + return nil + }, + } + + blockedCmd.Flags().StringVarP(&ops.serviceId, "serviceId", "", "", "Name of the SaaS service file (without extension)") + blockedCmd.Flags().StringVarP(&ops.componentName, "component", "c", "", "Name of the code component in app.yaml") + blockedCmd.Flags().StringVarP(&ops.gitHash, "gitHash", "g", "", "SHA commit hash to add to blockedVersions") + blockedCmd.Flags().StringVarP(&ops.appInterfaceProvidedPath, "appInterfaceDir", "", "", "Location of app-interface checkout. Falls back to the current working directory") + + return blockedCmd +} diff --git a/cmd/promote/cmd.go b/cmd/promote/cmd.go index 4676a4bbd..7f2727e54 100644 --- a/cmd/promote/cmd.go +++ b/cmd/promote/cmd.go @@ -3,6 +3,7 @@ package promote import ( "fmt" + "github.com/openshift/osdctl/cmd/promote/blocked" "github.com/openshift/osdctl/cmd/promote/dynatrace" "github.com/openshift/osdctl/cmd/promote/managedscripts" "github.com/openshift/osdctl/cmd/promote/saas" @@ -21,6 +22,7 @@ func NewCmdPromote() *cobra.Command { promoteCmd.AddCommand(saas.NewCmdSaas()) promoteCmd.AddCommand(dynatrace.NewCmdDynatrace()) promoteCmd.AddCommand(managedscripts.NewCmdManagedScripts()) + promoteCmd.AddCommand(blocked.NewCmdBlocked()) return promoteCmd } diff --git a/pkg/promote/service.go b/pkg/promote/service.go index 9f9ab6551..e90523c0f 100644 --- a/pkg/promote/service.go +++ b/pkg/promote/service.go @@ -77,6 +77,40 @@ func (c *CodeComponent) SetHotfixVersion(hotfixVersion string) error { return nil } +func (c *CodeComponent) AddBlockedVersion(blockedVersion string) error { + existingNode, err := kyaml.Lookup("blockedVersions").Filter(c.node) + if err != nil { + return fmt.Errorf("failed to lookup 'codeComponents[].blockedVersions' in '%s': %v", c.filePath, err) + } + + if existingNode != nil { + elements, err := existingNode.Elements() + if err != nil { + return fmt.Errorf("failed to read 'codeComponents[].blockedVersions' in '%s': %v", c.filePath, err) + } + for _, elem := range elements { + val, err := elem.String() + if err != nil { + return fmt.Errorf("invalid non-string value in 'codeComponents[].blockedVersions' in '%s': %v", c.filePath, err) + } + val = strings.TrimSpace(val) + if val == blockedVersion { + return fmt.Errorf("version '%s' is already in 'codeComponents[].blockedVersions' in '%s'", blockedVersion, c.filePath) + } + } + err = existingNode.PipeE(kyaml.Append(kyaml.NewStringRNode(blockedVersion).YNode())) + if err != nil { + return fmt.Errorf("failed to append to 'codeComponents[].blockedVersions' in '%s': %v", c.filePath, err) + } + } else { + _, err = kyaml.SetField("blockedVersions", kyaml.NewListRNode(blockedVersion)).Filter(c.node) + if err != nil { + return fmt.Errorf("failed to set 'codeComponents[].blockedVersions' in '%s': %v", c.filePath, err) + } + } + return nil +} + type Application struct { yamlDoc componentsSequenceNode *kyaml.RNode @@ -125,6 +159,33 @@ func (a *Application) GetComponent(componentUrl string) (*CodeComponent, error) return newCodeComponent(a.filePath, componentNode) } +func (a *Application) GetComponentByName(componentName string) (*CodeComponent, error) { + var componentNode *kyaml.RNode + + err := a.componentsSequenceNode.VisitElements(func(visitedNode *kyaml.RNode) error { + visitedName, err := visitedNode.GetString("name") + if err != nil { + return fmt.Errorf("path 'codeComponents[].name' is not always defined as a string in '%s': %v", a.filePath, err) + } + if visitedName == componentName { + if componentNode != nil { + return fmt.Errorf("path 'codeComponents[].name' is defined to '%s' more than once in '%s'", componentName, a.filePath) + } + componentNode = visitedNode + } + return nil + }) + if err != nil { + return nil, fmt.Errorf("failed to iterate over 'codeComponents' in '%s': %v", a.filePath, err) + } + + if componentNode == nil { + return nil, fmt.Errorf("component '%s' not found in '%s'", componentName, a.filePath) + } + + return newCodeComponent(a.filePath, componentNode) +} + type Service struct { yamlDoc appInterfaceClone *AppInterfaceClone diff --git a/pkg/promote/service_test.go b/pkg/promote/service_test.go index dbfb72fd0..1bb2d5b0a 100644 --- a/pkg/promote/service_test.go +++ b/pkg/promote/service_test.go @@ -109,6 +109,165 @@ var _ = Describe("Application struct", func() { }) }) +var _ = Describe("Application.GetComponentByName", func() { + var data *TestData + + BeforeEach(func() { + data = CreateDefaultTestData() + }) + + AfterEach(func() { + CleanupAllTestDataResources() + }) + + It("returns the component when found by name", func() { + application, err := readApplicationFromFile(filepath.Join(data.AppInterfacePath, "data/services/gen-app/app.yml")) + Expect(err).ShouldNot(HaveOccurred()) + + component, err := application.GetComponentByName("default-component") + Expect(err).ShouldNot(HaveOccurred()) + Expect(component).ToNot(BeNil()) + Expect(component.GetName()).To(Equal("default-component")) + }) + + It("returns the dummy-component when found by name", func() { + application, err := readApplicationFromFile(filepath.Join(data.AppInterfacePath, "data/services/gen-app/app.yml")) + Expect(err).ShouldNot(HaveOccurred()) + + component, err := application.GetComponentByName("dummy-component") + Expect(err).ShouldNot(HaveOccurred()) + Expect(component).ToNot(BeNil()) + Expect(component.GetName()).To(Equal("dummy-component")) + }) + + It("returns an error when the component name does not exist", func() { + application, err := readApplicationFromFile(filepath.Join(data.AppInterfacePath, "data/services/gen-app/app.yml")) + Expect(err).ShouldNot(HaveOccurred()) + + component, err := application.GetComponentByName("nonexistent-component") + Expect(err).Should(HaveOccurred()) + Expect(component).To(BeNil()) + Expect(err.Error()).To(ContainSubstring("nonexistent-component")) + }) +}) + +var _ = Describe("CodeComponent.AddBlockedVersion", func() { + var data *TestData + + BeforeEach(func() { + data = CreateDefaultTestData() + }) + + AfterEach(func() { + CleanupAllTestDataResources() + }) + + Context("when no blockedVersions field exists yet", func() { + It("creates the field with a single entry", func() { + application, err := readApplicationFromFile(filepath.Join(data.AppInterfacePath, "data/services/gen-app/app.yml")) + Expect(err).ShouldNot(HaveOccurred()) + + component, err := application.GetComponentByName("default-component") + Expect(err).ShouldNot(HaveOccurred()) + + err = component.AddBlockedVersion("abc123") + Expect(err).ShouldNot(HaveOccurred()) + + err = application.Save() + Expect(err).ShouldNot(HaveOccurred()) + + expectedProperties := InitProperties(data.TestRepoPath, "") + expectedProperties["blockedVersion"] = "abc123" + expectedAppFileContent := GetFileContent(AppFileContentTemplateWithBlockedVersion, "gen-app", expectedProperties) + Expect(data.ReadAppInterfaceFile("data/services/gen-app/app.yml")).To(Equal(expectedAppFileContent)) + }) + }) + + Context("when blockedVersions already has one entry", func() { + BeforeEach(func() { + properties := InitProperties(data.TestRepoPath, "") + properties["blockedVersion"] = "existing123" + appFileContent := GetFileContent(AppFileContentTemplateWithBlockedVersion, "gen-app", properties) + data.WriteAppInterfaceFile("data/services/gen-app/app.yml", appFileContent) + }) + + It("appends a second entry", func() { + application, err := readApplicationFromFile(filepath.Join(data.AppInterfacePath, "data/services/gen-app/app.yml")) + Expect(err).ShouldNot(HaveOccurred()) + + component, err := application.GetComponentByName("default-component") + Expect(err).ShouldNot(HaveOccurred()) + + err = component.AddBlockedVersion("newblock456") + Expect(err).ShouldNot(HaveOccurred()) + + err = application.Save() + Expect(err).ShouldNot(HaveOccurred()) + + expectedProperties := InitProperties(data.TestRepoPath, "") + expectedProperties["blockedVersion1"] = "existing123" + expectedProperties["blockedVersion2"] = "newblock456" + expectedAppFileContent := GetFileContent(AppFileContentTemplateWithBlockedVersions, "gen-app", expectedProperties) + Expect(data.ReadAppInterfaceFile("data/services/gen-app/app.yml")).To(Equal(expectedAppFileContent)) + }) + + It("returns an error when the version already exists", func() { + application, err := readApplicationFromFile(filepath.Join(data.AppInterfacePath, "data/services/gen-app/app.yml")) + Expect(err).ShouldNot(HaveOccurred()) + + component, err := application.GetComponentByName("default-component") + Expect(err).ShouldNot(HaveOccurred()) + + err = component.AddBlockedVersion("existing123") + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("already in")) + Expect(err.Error()).To(ContainSubstring("existing123")) + }) + }) + + Context("when blockedVersions already has two entries", func() { + BeforeEach(func() { + properties := InitProperties(data.TestRepoPath, "") + properties["blockedVersion1"] = "first111" + properties["blockedVersion2"] = "second222" + appFileContent := GetFileContent(AppFileContentTemplateWithBlockedVersions, "gen-app", properties) + data.WriteAppInterfaceFile("data/services/gen-app/app.yml", appFileContent) + }) + + It("appends a third entry", func() { + application, err := readApplicationFromFile(filepath.Join(data.AppInterfacePath, "data/services/gen-app/app.yml")) + Expect(err).ShouldNot(HaveOccurred()) + + component, err := application.GetComponentByName("default-component") + Expect(err).ShouldNot(HaveOccurred()) + + err = component.AddBlockedVersion("third333") + Expect(err).ShouldNot(HaveOccurred()) + + err = application.Save() + Expect(err).ShouldNot(HaveOccurred()) + + // Read back and verify all three are present + actualContent := data.ReadAppInterfaceFile("data/services/gen-app/app.yml") + Expect(actualContent).To(ContainSubstring("first111")) + Expect(actualContent).To(ContainSubstring("second222")) + Expect(actualContent).To(ContainSubstring("third333")) + }) + + It("rejects a duplicate of the first entry", func() { + application, err := readApplicationFromFile(filepath.Join(data.AppInterfacePath, "data/services/gen-app/app.yml")) + Expect(err).ShouldNot(HaveOccurred()) + + component, err := application.GetComponentByName("default-component") + Expect(err).ShouldNot(HaveOccurred()) + + err = component.AddBlockedVersion("second222") + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("already in")) + }) + }) +}) + var _ = Describe("Service struct", func() { var data *TestData var service *Service diff --git a/pkg/promote/test_tools.go b/pkg/promote/test_tools.go index 9ba3810d8..68d0af942 100644 --- a/pkg/promote/test_tools.go +++ b/pkg/promote/test_tools.go @@ -66,6 +66,13 @@ codeComponents: AppFileContentTemplateWithHotfixVersions = AppFileContentTemplate + ` hotfixVersions: - @hotfixVersion1@ - @hotfixVersion2@ +` + AppFileContentTemplateWithBlockedVersion = AppFileContentTemplate + ` blockedVersions: + - @blockedVersion@ +` + AppFileContentTemplateWithBlockedVersions = AppFileContentTemplate + ` blockedVersions: + - @blockedVersion1@ + - @blockedVersion2@ ` ) From 3cc70ba319284307c8180ebc0684e41a633ff497 Mon Sep 17 00:00:00 2001 From: Florian Bergmann Date: Mon, 11 May 2026 12:46:06 +0200 Subject: [PATCH 3/6] feat: add --list flag to blocked command to show available components Add Application.GetComponentNames() to pkg/promote which returns all component names from the app.yaml codeComponents array. The blocked command now supports: osdctl promote blocked --serviceId --list This lets users discover available component names without needing to manually inspect app-interface. The --list flag is mutually exclusive with --component and --gitHash. The --component error message now also hints at --list: --component is required (use --list to see available components) The --list path skips the clean-check on the app-interface clone since it is a read-only operation. --- cmd/promote/blocked/blocked.go | 65 +++++++++++++++++++++++++--------- pkg/promote/service.go | 18 ++++++++++ pkg/promote/service_test.go | 12 +++++++ 3 files changed, 79 insertions(+), 16 deletions(-) diff --git a/cmd/promote/blocked/blocked.go b/cmd/promote/blocked/blocked.go index 2771d3d95..90b8f55bd 100644 --- a/cmd/promote/blocked/blocked.go +++ b/cmd/promote/blocked/blocked.go @@ -10,6 +10,8 @@ import ( ) type blockedOptions struct { + list bool + appInterfaceProvidedPath string serviceId string componentName string @@ -35,20 +37,29 @@ Duplicate entries are rejected with an error.`, Args: cobra.NoArgs, DisableAutoGenTag: true, Example: ` + # List all services and their components + osdctl promote blocked --list + # Block a specific version for a component osdctl promote blocked --serviceId --component --gitHash # With explicit app-interface path osdctl promote blocked --serviceId --component --gitHash --appInterfaceDir /path/to/app-interface`, RunE: func(cmd *cobra.Command, args []string) error { - if ops.serviceId == "" { - return fmt.Errorf("--serviceId is required") - } - if ops.componentName == "" { - return fmt.Errorf("--component is required") - } - if ops.gitHash == "" { - return fmt.Errorf("--gitHash is required") + if ops.list { + if ops.serviceId != "" || ops.componentName != "" || ops.gitHash != "" { + return fmt.Errorf("--list cannot be used with --serviceId, --component or --gitHash") + } + } else { + if ops.serviceId == "" { + return fmt.Errorf("--serviceId is required (use --list to see available services and components)") + } + if ops.componentName == "" { + return fmt.Errorf("--component is required (use --list to see available services and components)") + } + if ops.gitHash == "" { + return fmt.Errorf("--gitHash is required") + } } cmd.SilenceUsage = true @@ -58,14 +69,6 @@ Duplicate entries are rejected with an error.`, return err } - isClean, err := appInterfaceClone.IsClean() - if err != nil { - return err - } - if !isClean { - return fmt.Errorf("app-interface clone in '%s' has uncommitted changes, please commit or stash them before proceeding", appInterfaceClone.GetPath()) - } - servicesRegistry, err := promote.NewServicesRegistry( appInterfaceClone, func(filePath string) string { return filePath }, @@ -77,6 +80,27 @@ Duplicate entries are rejected with an error.`, return err } + if ops.list { + fmt.Println("### Services and their components ###") + for _, serviceId := range servicesRegistry.GetServicesIds() { + service, err := servicesRegistry.GetService(serviceId) + if err != nil { + fmt.Printf(" %s (error: %v)\n", serviceId, err) + continue + } + componentNames, err := service.GetApplication().GetComponentNames() + if err != nil { + fmt.Printf(" %s (error reading components: %v)\n", serviceId, err) + continue + } + fmt.Printf(" %s\n", serviceId) + for _, name := range componentNames { + fmt.Printf(" - %s\n", name) + } + } + return nil + } + service, err := servicesRegistry.GetService(ops.serviceId) if err != nil { return err @@ -84,6 +108,14 @@ Duplicate entries are rejected with an error.`, application := service.GetApplication() + isClean, err := appInterfaceClone.IsClean() + if err != nil { + return err + } + if !isClean { + return fmt.Errorf("app-interface clone in '%s' has uncommitted changes, please commit or stash them before proceeding", appInterfaceClone.GetPath()) + } + component, err := application.GetComponentByName(ops.componentName) if err != nil { return err @@ -137,6 +169,7 @@ Duplicate entries are rejected with an error.`, }, } + blockedCmd.Flags().BoolVarP(&ops.list, "list", "l", false, "List all services and their components") blockedCmd.Flags().StringVarP(&ops.serviceId, "serviceId", "", "", "Name of the SaaS service file (without extension)") blockedCmd.Flags().StringVarP(&ops.componentName, "component", "c", "", "Name of the code component in app.yaml") blockedCmd.Flags().StringVarP(&ops.gitHash, "gitHash", "g", "", "SHA commit hash to add to blockedVersions") diff --git a/pkg/promote/service.go b/pkg/promote/service.go index e90523c0f..3b48205bd 100644 --- a/pkg/promote/service.go +++ b/pkg/promote/service.go @@ -159,6 +159,24 @@ func (a *Application) GetComponent(componentUrl string) (*CodeComponent, error) return newCodeComponent(a.filePath, componentNode) } +func (a *Application) GetComponentNames() ([]string, error) { + var names []string + + err := a.componentsSequenceNode.VisitElements(func(visitedNode *kyaml.RNode) error { + name, err := visitedNode.GetString("name") + if err != nil || name == "" { + return fmt.Errorf("path 'codeComponents[].name' is not always defined as a non-empty string in '%s': %v", a.filePath, err) + } + names = append(names, name) + return nil + }) + if err != nil { + return nil, fmt.Errorf("failed to iterate over 'codeComponents' in '%s': %v", a.filePath, err) + } + + return names, nil +} + func (a *Application) GetComponentByName(componentName string) (*CodeComponent, error) { var componentNode *kyaml.RNode diff --git a/pkg/promote/service_test.go b/pkg/promote/service_test.go index 1bb2d5b0a..80c1b3ee4 100644 --- a/pkg/promote/service_test.go +++ b/pkg/promote/service_test.go @@ -61,6 +61,18 @@ var _ = Describe("Application struct", func() { }) }) + Context("Using GetComponentNames", func() { + It("returns all component names in order", func() { + application, err := readApplicationFromFile(filepath.Join(data.AppInterfacePath, "data/services/gen-app/app.yml")) + Expect(err).ShouldNot(HaveOccurred()) + Expect(application).ToNot(BeNil()) + + names, err := application.GetComponentNames() + Expect(err).ShouldNot(HaveOccurred()) + Expect(names).To(Equal([]string{"dummy-component", "default-component"})) + }) + }) + Context("Using SetHotfixVersion and Save", func() { var hotfixVersion string From 35fd6b3f06e6d33b34a1ccae874cbb1224ff19bc Mon Sep 17 00:00:00 2001 From: Florian Bergmann Date: Mon, 11 May 2026 13:04:39 +0200 Subject: [PATCH 4/6] feat: add --all flag to block a version across all components of a service Add Application.GetAllComponents() to pkg/promote which returns all CodeComponent instances from the app.yaml codeComponents array. The blocked command now supports: osdctl promote blocked --serviceId --all --gitHash This blocks the given SHA in every component of the service's app.yaml in a single commit, instead of requiring one invocation per component. --all and --component are mutually exclusive, enforced both by manual validation and cobra's MarkFlagsMutuallyExclusive. Commit messages adapt to the mode: --component: 'Block version for ' --all: 'Block version for all components of ' Includes a new unit test for GetAllComponents. --- cmd/promote/blocked/blocked.go | 75 +++++++++++++++++++++++++--------- pkg/promote/service.go | 18 ++++++++ pkg/promote/service_test.go | 14 +++++++ 3 files changed, 87 insertions(+), 20 deletions(-) diff --git a/cmd/promote/blocked/blocked.go b/cmd/promote/blocked/blocked.go index 90b8f55bd..f7269a2b1 100644 --- a/cmd/promote/blocked/blocked.go +++ b/cmd/promote/blocked/blocked.go @@ -11,6 +11,7 @@ import ( type blockedOptions struct { list bool + all bool appInterfaceProvidedPath string serviceId string @@ -40,22 +41,28 @@ Duplicate entries are rejected with an error.`, # List all services and their components osdctl promote blocked --list - # Block a specific version for a component + # Block a specific version for a single component osdctl promote blocked --serviceId --component --gitHash + # Block a specific version for all components of a service + osdctl promote blocked --serviceId --all --gitHash + # With explicit app-interface path osdctl promote blocked --serviceId --component --gitHash --appInterfaceDir /path/to/app-interface`, RunE: func(cmd *cobra.Command, args []string) error { if ops.list { - if ops.serviceId != "" || ops.componentName != "" || ops.gitHash != "" { - return fmt.Errorf("--list cannot be used with --serviceId, --component or --gitHash") + if ops.serviceId != "" || ops.componentName != "" || ops.gitHash != "" || ops.all { + return fmt.Errorf("--list cannot be used with --serviceId, --component, --all or --gitHash") } } else { if ops.serviceId == "" { return fmt.Errorf("--serviceId is required (use --list to see available services and components)") } - if ops.componentName == "" { - return fmt.Errorf("--component is required (use --list to see available services and components)") + if ops.all && ops.componentName != "" { + return fmt.Errorf("--all and --component are mutually exclusive") + } + if !ops.all && ops.componentName == "" { + return fmt.Errorf("--component or --all is required (use --list to see available services and components)") } if ops.gitHash == "" { return fmt.Errorf("--gitHash is required") @@ -116,14 +123,28 @@ Duplicate entries are rejected with an error.`, return fmt.Errorf("app-interface clone in '%s' has uncommitted changes, please commit or stash them before proceeding", appInterfaceClone.GetPath()) } - component, err := application.GetComponentByName(ops.componentName) - if err != nil { - return err + var components []*promote.CodeComponent + + if ops.all { + components, err = application.GetAllComponents() + if err != nil { + return err + } + } else { + component, err := application.GetComponentByName(ops.componentName) + if err != nil { + return err + } + components = []*promote.CodeComponent{component} } - err = component.AddBlockedVersion(ops.gitHash) - if err != nil { - return err + var blockedNames []string + for _, component := range components { + err = component.AddBlockedVersion(ops.gitHash) + if err != nil { + return err + } + blockedNames = append(blockedNames, component.GetName()) } err = application.Save() @@ -131,19 +152,31 @@ Duplicate entries are rejected with an error.`, return fmt.Errorf("failed to save application '%s': %v", application.GetFilePath(), err) } - branchName := fmt.Sprintf("block-%s-%s", ops.componentName, ops.gitHash) + targetLabel := strings.Join(blockedNames, ", ") + branchName := fmt.Sprintf("block-%s-%s", ops.serviceId, ops.gitHash) err = appInterfaceClone.CheckoutNewBranch(branchName) if err != nil { return err } - commitMessage := fmt.Sprintf("Block version %s for %s\n\nAdd %s to blockedVersions for component '%s' in '%s'.", - ops.gitHash, - ops.componentName, - ops.gitHash, - ops.componentName, - filepath.Base(application.GetFilePath()), - ) + var commitMessage string + if ops.all { + commitMessage = fmt.Sprintf("Block version %s for all components of %s\n\nAdd %s to blockedVersions for components [%s] in '%s'.", + ops.gitHash, + ops.serviceId, + ops.gitHash, + targetLabel, + filepath.Base(application.GetFilePath()), + ) + } else { + commitMessage = fmt.Sprintf("Block version %s for %s\n\nAdd %s to blockedVersions for component '%s' in '%s'.", + ops.gitHash, + ops.componentName, + ops.gitHash, + ops.componentName, + filepath.Base(application.GetFilePath()), + ) + } err = appInterfaceClone.Commit(commitMessage) if err != nil { @@ -151,7 +184,7 @@ Duplicate entries are rejected with an error.`, } fmt.Println("SUCCESS!") - fmt.Printf("Blocked version %s for component %s\n", ops.gitHash, ops.componentName) + fmt.Printf("Blocked version %s for: %s\n", ops.gitHash, targetLabel) fmt.Printf("Application file: %s\n", application.GetFilePath()) fmt.Println("") fmt.Println("------------- Commit message -------------") @@ -170,10 +203,12 @@ Duplicate entries are rejected with an error.`, } blockedCmd.Flags().BoolVarP(&ops.list, "list", "l", false, "List all services and their components") + blockedCmd.Flags().BoolVarP(&ops.all, "all", "a", false, "Block the version for all components of the service (mutually exclusive with --component)") blockedCmd.Flags().StringVarP(&ops.serviceId, "serviceId", "", "", "Name of the SaaS service file (without extension)") blockedCmd.Flags().StringVarP(&ops.componentName, "component", "c", "", "Name of the code component in app.yaml") blockedCmd.Flags().StringVarP(&ops.gitHash, "gitHash", "g", "", "SHA commit hash to add to blockedVersions") blockedCmd.Flags().StringVarP(&ops.appInterfaceProvidedPath, "appInterfaceDir", "", "", "Location of app-interface checkout. Falls back to the current working directory") + blockedCmd.MarkFlagsMutuallyExclusive("all", "component") return blockedCmd } diff --git a/pkg/promote/service.go b/pkg/promote/service.go index 3b48205bd..6b1e14c37 100644 --- a/pkg/promote/service.go +++ b/pkg/promote/service.go @@ -177,6 +177,24 @@ func (a *Application) GetComponentNames() ([]string, error) { return names, nil } +func (a *Application) GetAllComponents() ([]*CodeComponent, error) { + var components []*CodeComponent + + err := a.componentsSequenceNode.VisitElements(func(visitedNode *kyaml.RNode) error { + component, err := newCodeComponent(a.filePath, visitedNode) + if err != nil { + return err + } + components = append(components, component) + return nil + }) + if err != nil { + return nil, fmt.Errorf("failed to iterate over 'codeComponents' in '%s': %v", a.filePath, err) + } + + return components, nil +} + func (a *Application) GetComponentByName(componentName string) (*CodeComponent, error) { var componentNode *kyaml.RNode diff --git a/pkg/promote/service_test.go b/pkg/promote/service_test.go index 80c1b3ee4..f65514c75 100644 --- a/pkg/promote/service_test.go +++ b/pkg/promote/service_test.go @@ -73,6 +73,20 @@ var _ = Describe("Application struct", func() { }) }) + Context("Using GetAllComponents", func() { + It("returns all components in order", func() { + application, err := readApplicationFromFile(filepath.Join(data.AppInterfacePath, "data/services/gen-app/app.yml")) + Expect(err).ShouldNot(HaveOccurred()) + Expect(application).ToNot(BeNil()) + + components, err := application.GetAllComponents() + Expect(err).ShouldNot(HaveOccurred()) + Expect(components).To(HaveLen(2)) + Expect(components[0].GetName()).To(Equal("dummy-component")) + Expect(components[1].GetName()).To(Equal("default-component")) + }) + }) + Context("Using SetHotfixVersion and Save", func() { var hotfixVersion string From 4286ac57d641c42e93bc9f4f8bbd69aa5e28ca09 Mon Sep 17 00:00:00 2001 From: Florian Bergmann Date: Mon, 11 May 2026 15:46:27 +0200 Subject: [PATCH 5/6] fix: rename CLI command to 'block' and checkout branch before mutations - Rename 'osdctl promote blocked' to 'osdctl promote block' - Rename NewCmdBlocked() to NewCmdBlock() for consistency - Move CheckoutNewBranch() before any YAML modifications so the branch is created on a clean state and all file changes happen on the new branch - Update all example strings to use the new command name --- cmd/promote/blocked/blocked.go | 25 +++++++++++++------------ cmd/promote/cmd.go | 2 +- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/cmd/promote/blocked/blocked.go b/cmd/promote/blocked/blocked.go index f7269a2b1..3d2c9845d 100644 --- a/cmd/promote/blocked/blocked.go +++ b/cmd/promote/blocked/blocked.go @@ -19,11 +19,11 @@ type blockedOptions struct { gitHash string } -// NewCmdBlocked implements the blocked command to add a blocked version to a component in app.yaml -func NewCmdBlocked() *cobra.Command { +// NewCmdBlock implements the block command to add a blocked version to a component in app.yaml +func NewCmdBlock() *cobra.Command { ops := &blockedOptions{} blockedCmd := &cobra.Command{ - Use: "blocked", + Use: "block", Short: "Add a blocked version to a component in app.yaml", Long: `Add a SHA commit hash to the blockedVersions list for a code component in the application's app.yaml file. This prevents the specified version @@ -39,16 +39,16 @@ Duplicate entries are rejected with an error.`, DisableAutoGenTag: true, Example: ` # List all services and their components - osdctl promote blocked --list + osdctl promote block --list # Block a specific version for a single component - osdctl promote blocked --serviceId --component --gitHash + osdctl promote block --serviceId --component --gitHash # Block a specific version for all components of a service - osdctl promote blocked --serviceId --all --gitHash + osdctl promote block --serviceId --all --gitHash # With explicit app-interface path - osdctl promote blocked --serviceId --component --gitHash --appInterfaceDir /path/to/app-interface`, + osdctl promote block --serviceId --component --gitHash --appInterfaceDir /path/to/app-interface`, RunE: func(cmd *cobra.Command, args []string) error { if ops.list { if ops.serviceId != "" || ops.componentName != "" || ops.gitHash != "" || ops.all { @@ -123,6 +123,12 @@ Duplicate entries are rejected with an error.`, return fmt.Errorf("app-interface clone in '%s' has uncommitted changes, please commit or stash them before proceeding", appInterfaceClone.GetPath()) } + branchName := fmt.Sprintf("block-%s-%s", ops.serviceId, ops.gitHash) + err = appInterfaceClone.CheckoutNewBranch(branchName) + if err != nil { + return err + } + var components []*promote.CodeComponent if ops.all { @@ -153,11 +159,6 @@ Duplicate entries are rejected with an error.`, } targetLabel := strings.Join(blockedNames, ", ") - branchName := fmt.Sprintf("block-%s-%s", ops.serviceId, ops.gitHash) - err = appInterfaceClone.CheckoutNewBranch(branchName) - if err != nil { - return err - } var commitMessage string if ops.all { diff --git a/cmd/promote/cmd.go b/cmd/promote/cmd.go index 7f2727e54..689c592d4 100644 --- a/cmd/promote/cmd.go +++ b/cmd/promote/cmd.go @@ -22,7 +22,7 @@ func NewCmdPromote() *cobra.Command { promoteCmd.AddCommand(saas.NewCmdSaas()) promoteCmd.AddCommand(dynatrace.NewCmdDynatrace()) promoteCmd.AddCommand(managedscripts.NewCmdManagedScripts()) - promoteCmd.AddCommand(blocked.NewCmdBlocked()) + promoteCmd.AddCommand(blocked.NewCmdBlock()) return promoteCmd } From 159aae48fbcca4aef74be5faaed8aa85af29fae9 Mon Sep 17 00:00:00 2001 From: Florian Bergmann Date: Mon, 11 May 2026 15:53:31 +0200 Subject: [PATCH 6/6] Add documentation for the new block command. --- docs/README.md | 31 +++++++++++++++++++ docs/osdctl_promote.md | 1 + docs/osdctl_promote_block.md | 60 ++++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+) create mode 100644 docs/osdctl_promote_block.md diff --git a/docs/README.md b/docs/README.md index bbaa4e80c..76b8c93d1 100644 --- a/docs/README.md +++ b/docs/README.md @@ -135,6 +135,7 @@ - `labels` - get organization labels - `users` - get organization users - `promote` - Utilities to promote services/operators + - `block` - Add a blocked version to a component in app.yaml - `dynatrace` - Utilities to promote dynatrace - `managedscripts` - Promote https://github.com/openshift/managed-scripts - `saas` - Utilities to promote SaaS services/operators @@ -4165,6 +4166,36 @@ osdctl promote [flags] -S, --skip-version-check skip checking to see if this is the most recent release ``` +### osdctl promote block + +Add a SHA commit hash to the blockedVersions list for a code component +in the application's app.yaml file. This prevents the specified version +from being promoted through progressive delivery. + +The command locates the app.yaml through the SaaS service file, finds +the specified component by name, and appends the git hash to its +codeComponents[].blockedVersions array. If the blockedVersions field +does not yet exist, it will be created. + +Duplicate entries are rejected with an error. + +``` +osdctl promote block [flags] +``` + +#### Flags + +``` + -a, --all Block the version for all components of the service (mutually exclusive with --component) + --appInterfaceDir string Location of app-interface checkout. Falls back to the current working directory + -c, --component string Name of the code component in app.yaml + -g, --gitHash string SHA commit hash to add to blockedVersions + -h, --help help for block + -l, --list List all services and their components + --serviceId string Name of the SaaS service file (without extension) + -S, --skip-version-check skip checking to see if this is the most recent release +``` + ### osdctl promote dynatrace Promote Dynatrace components or terraform modules. diff --git a/docs/osdctl_promote.md b/docs/osdctl_promote.md index 3b974c2c2..8370103e0 100644 --- a/docs/osdctl_promote.md +++ b/docs/osdctl_promote.md @@ -17,6 +17,7 @@ Utilities to promote services/operators ### SEE ALSO * [osdctl](osdctl.md) - OSD CLI +* [osdctl promote block](osdctl_promote_block.md) - Add a blocked version to a component in app.yaml * [osdctl promote dynatrace](osdctl_promote_dynatrace.md) - Utilities to promote dynatrace * [osdctl promote managedscripts](osdctl_promote_managedscripts.md) - Promote https://github.com/openshift/managed-scripts * [osdctl promote saas](osdctl_promote_saas.md) - Utilities to promote SaaS services/operators diff --git a/docs/osdctl_promote_block.md b/docs/osdctl_promote_block.md new file mode 100644 index 000000000..938faf2f4 --- /dev/null +++ b/docs/osdctl_promote_block.md @@ -0,0 +1,60 @@ +## osdctl promote block + +Add a blocked version to a component in app.yaml + +### Synopsis + +Add a SHA commit hash to the blockedVersions list for a code component +in the application's app.yaml file. This prevents the specified version +from being promoted through progressive delivery. + +The command locates the app.yaml through the SaaS service file, finds +the specified component by name, and appends the git hash to its +codeComponents[].blockedVersions array. If the blockedVersions field +does not yet exist, it will be created. + +Duplicate entries are rejected with an error. + +``` +osdctl promote block [flags] +``` + +### Examples + +``` + + # List all services and their components + osdctl promote block --list + + # Block a specific version for a single component + osdctl promote block --serviceId --component --gitHash + + # Block a specific version for all components of a service + osdctl promote block --serviceId --all --gitHash + + # With explicit app-interface path + osdctl promote block --serviceId --component --gitHash --appInterfaceDir /path/to/app-interface +``` + +### Options + +``` + -a, --all Block the version for all components of the service (mutually exclusive with --component) + --appInterfaceDir string Location of app-interface checkout. Falls back to the current working directory + -c, --component string Name of the code component in app.yaml + -g, --gitHash string SHA commit hash to add to blockedVersions + -h, --help help for block + -l, --list List all services and their components + --serviceId string Name of the SaaS service file (without extension) +``` + +### Options inherited from parent commands + +``` + -S, --skip-version-check skip checking to see if this is the most recent release +``` + +### SEE ALSO + +* [osdctl promote](osdctl_promote.md) - Utilities to promote services/operators +