OCM-11480 | fix: not exit when user choosing N for registry config

Signed-off-by: Maggie Chen <[email protected]> fix lint Signed-off-by: Maggie Chen <[email protected]> change text Signed-off-by: Maggie Chen <[email protected]> OCM-10017 | fix: fix create account role test change based on Philip's PR Signed-off-by: Maggie Chen <[email protected]> add coverage Signed-off-by: Maggie Chen <[email protected]>
openshift · Oct 17, 2024 · fb37f8c · fb37f8c
1 parent 94564fe
commit fb37f8c
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 18 deletions.
diff --git a/cmd/create/accountroles/creators_test.go b/cmd/create/accountroles/creators_test.go
@@ -27,7 +27,7 @@ var _ = Describe("Accountroles", Ordered, func() {
 			accountRolesCreationInput := buildRolesCreationInput("test", "", "account-123", "stage", policies, "", "")
 			err := (&hcpManagedPoliciesCreator{}).createRoles(r, accountRolesCreationInput)
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(Equal("failed to find policy ARN for 'sts_hcp_installer_permission_policy'"))
+			Expect(err.Error()).To(ContainSubstring("failed to find policy ARN for"))
 		})
 		It("createRole succeeds", func() {
 			mockCtrl := gomock.NewController(GinkgoT())

diff --git a/cmd/edit/cluster/cmd.go b/cmd/edit/cluster/cmd.go
@@ -658,28 +658,16 @@ func run(cmd *cobra.Command, _ []string) {
 		// prompt for a warning if any registry config field is set
 		if allowedRegistries != nil || blockedRegistries != nil || insecureRegistries != nil ||
 			additionalTrustedCa != "" || allowedRegistriesForImport != "" || platformAllowlist != "" {
-			prompt := "Changing any registry related parameter will trigger a rollout across all machinepools " +
-				"(all machinepool nodes will be recreated, following pod draining from each node). Do you want to proceed?"
-			if !confirm.ConfirmRaw(prompt) {
-				r.Reporter.Warnf("You have not changed any registry configuration -- exiting.")
-				os.Exit(0)
+			if PromptUserToAcceptRegistryChange(r) {
+				clusterConfig, err = BuildClusterConfigWithRegistry(clusterConfig, allowedRegistries,
+					blockedRegistries, insecureRegistries,
+					additionalTrustedCa, allowedRegistriesForImport, platformAllowlist)
 			}
-		}
-
-		clusterConfig.AllowedRegistries = allowedRegistries
-		clusterConfig.BlockedRegistries = blockedRegistries
-		clusterConfig.InsecureRegistries = insecureRegistries
-		clusterConfig.PlatformAllowlist = platformAllowlist
-		if additionalTrustedCa != "" {
-			ca, err := clusterregistryconfig.BuildAdditionalTrustedCAFromInputFile(additionalTrustedCa)
 			if err != nil {
-				r.Reporter.Errorf("Failed to build the additional trusted ca from file %s, got error: %s", additionalTrustedCa, err)
+				r.Reporter.Errorf("%s", err)
 				os.Exit(1)
 			}
-			clusterConfig.AdditionalTrustedCa = ca
-			clusterConfig.AdditionalTrustedCaFile = additionalTrustedCa
 		}
-		clusterConfig.AllowedRegistriesForImport = allowedRegistriesForImport
 	}
 	if auditLogRole != nil {
 		clusterConfig.AuditLogRoleARN = new(string)
@@ -966,3 +954,34 @@ func auditLogInteractivePrompt(r *rosa.Runtime, cmd *cobra.Command, cluster *cmv
 	}
 	return
 }
+
+func PromptUserToAcceptRegistryChange(r *rosa.Runtime) bool {
+	prompt := "Changing any registry related parameter will trigger a rollout across all machinepools " +
+		"(all machinepool nodes will be recreated, following pod draining from each node). Do you want to proceed?"
+	if !confirm.ConfirmRaw(prompt) {
+		r.Reporter.Warnf("No changes to registry configuration.")
+		return false
+	}
+	return true
+}
+
+func BuildClusterConfigWithRegistry(clusterConfig ocm.Spec, allowedRegistries []string,
+	blockedRegistries []string, insecureRegistries []string, additionalTrustedCa string,
+	allowedRegistriesForImport string, platformAllowlist string) (ocm.Spec, error) {
+	clusterConfig.AllowedRegistries = allowedRegistries
+	clusterConfig.BlockedRegistries = blockedRegistries
+	clusterConfig.InsecureRegistries = insecureRegistries
+	clusterConfig.PlatformAllowlist = platformAllowlist
+	if additionalTrustedCa != "" {
+		ca, err := clusterregistryconfig.BuildAdditionalTrustedCAFromInputFile(additionalTrustedCa)
+		if err != nil {
+			return clusterConfig, fmt.Errorf(
+				"Failed to build the additional trusted ca from file %s, got error: %s",
+				additionalTrustedCa, err)
+		}
+		clusterConfig.AdditionalTrustedCa = ca
+		clusterConfig.AdditionalTrustedCaFile = additionalTrustedCa
+	}
+	clusterConfig.AllowedRegistriesForImport = allowedRegistriesForImport
+	return clusterConfig, nil
+}
diff --git a/cmd/edit/cluster/cmd_test.go b/cmd/edit/cluster/cmd_test.go
@@ -25,6 +25,7 @@ import (
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
 	. "github.com/openshift-online/ocm-sdk-go/testing"
 
+	"github.com/openshift/rosa/pkg/ocm"
 	"github.com/openshift/rosa/pkg/test"
 )
 
@@ -94,6 +95,24 @@ var _ = Describe("Edit cluster", func() {
 				ContainSubstring("warning string OAuth visibility will be affected by cluster visibility change"))
 		})
 	})
+
+	Context("BuildClusterConfigWithRegistry", func() {
+		clusterConfig := ocm.Spec{
+			Name: "test-cluster",
+		}
+		allowedRegistries := []string{"registry.io1", "registry.io2"}
+		It("OK: should pass with valid inputs", func() {
+			output, err := BuildClusterConfigWithRegistry(clusterConfig, allowedRegistries, nil, nil, "", "", "")
+			Expect(err).NotTo(HaveOccurred())
+			Expect(output.AllowedRegistries).To(Equal(allowedRegistries))
+		})
+		It("KO: should fail with error if ca file does not exist", func() {
+			_, err := BuildClusterConfigWithRegistry(clusterConfig, allowedRegistries, nil, nil, "not-exist", "", "")
+			Expect(err).To(MatchError("Failed to build the additional trusted ca from file not-exist, " +
+				"got error: expected a valid additional trusted certificate spec file:" +
+				" open not-exist: no such file or directory"))
+		})
+	})
 })
 
 func buildTestIngresses(total int, public int) []*cmv1.Ingress {