diff --git a/cmd/create/accountroles/cmd.go b/cmd/create/accountroles/cmd.go index 7366626561..6b90a2ac61 100644 --- a/cmd/create/accountroles/cmd.go +++ b/cmd/create/accountroles/cmd.go @@ -17,14 +17,12 @@ limitations under the License. package accountroles import ( + "fmt" "os" "strings" "github.com/spf13/cobra" - "github.com/openshift/rosa/cmd/login" - "github.com/openshift/rosa/cmd/verify/oc" - "github.com/openshift/rosa/cmd/verify/quota" "github.com/openshift/rosa/pkg/aws" "github.com/openshift/rosa/pkg/fedramp" "github.com/openshift/rosa/pkg/interactive" @@ -173,14 +171,21 @@ func init() { interactive.AddFlag(flags) } -func run(cmd *cobra.Command, argv []string) { - r := rosa.NewRuntime().WithAWS() - - mode, err := interactive.GetMode() +func run(cmd *cobra.Command, _ []string) { + r := rosa.NewRuntime().WithAWS().WithOCM() + defer r.Cleanup() + err := runWithRuntime(r, cmd) if err != nil { r.Reporter.Errorf("%s", err) os.Exit(1) } +} + +func runWithRuntime(r *rosa.Runtime, cmd *cobra.Command) error { + mode, err := interactive.GetMode() + if err != nil { + return err + } var isHcpSharedVpc bool if args.classic && !args.hostedCP { @@ -190,47 +195,31 @@ func run(cmd *cobra.Command, argv []string) { isHcpSharedVpc, err = roles.ValidateSharedVpcInputs(args.vpcEndpointRoleArn, args.route53RoleArn, vpcEndpointRoleArnFlag, route53RoleArnFlag) if err != nil { - r.Reporter.Errorf("%s", err) - os.Exit(1) + return err } } if args.vpcEndpointRoleArn != "" { err = aws.ARNValidator(args.vpcEndpointRoleArn) if err != nil { - r.Reporter.Errorf("Expected a valid policy ARN for %s: %s", vpcEndpointRoleArnFlag, err) - os.Exit(1) + return fmt.Errorf("expected a valid policy ARN for %s: %s", vpcEndpointRoleArnFlag, err) } } if args.route53RoleArn != "" { err = aws.ARNValidator(args.route53RoleArn) if err != nil { - r.Reporter.Errorf("Expected a valid policy ARN for %s: %s", route53RoleArnFlag, err) - os.Exit(1) + return fmt.Errorf("expected a valid policy ARN for %s: %s", route53RoleArnFlag, err) } } - // If necessary, call `login` as part of `init`. We do this before - // other validations to get the prompt out of the way before performing - // longer checks. - err = login.Call(cmd, argv, r.Reporter) - if err != nil { - r.Reporter.Errorf("Failed to login to OCM: %v", err) - os.Exit(1) - } - r.WithOCM() - defer r.Cleanup() - env, err := ocm.GetEnv() if err != nil { - r.Reporter.Errorf("Failed to determine OCM environment: %v", err) - os.Exit(1) + return fmt.Errorf("failed to determine OCM environment: %v", err) } managedPolicies := args.managed if args.forcePolicyCreation && managedPolicies { - r.Reporter.Warnf("Forcing creation of policies only works for unmanaged policies") - os.Exit(1) + return fmt.Errorf("forcing creation of policies only works for unmanaged policies") } if args.hostedCP && cmd.Flags().Changed("version") { @@ -252,14 +241,12 @@ func run(cmd *cobra.Command, argv []string) { isManagedSet = false managedPolicies = false } else { - r.Reporter.Errorf("Setting `hosted-cp` as unmanaged policies is not supported") - os.Exit(1) + return fmt.Errorf("setting `hosted-cp` as unmanaged policies is not supported") } } if roles.ClassicManagedPoliciesUnsupportedInEnv(isManagedSet, args.managed, env) { - r.Reporter.Errorf("Classic ROSA managed policies are not supported in this environment") - os.Exit(1) + return fmt.Errorf("classic ROSA managed policies are not supported in this environment") } // Validate AWS credentials for current user @@ -269,24 +256,16 @@ func run(cmd *cobra.Command, argv []string) { ok, err := r.AWSClient.ValidateCredentials() if err != nil { r.OCMClient.LogEvent("ROSAInitCredentialsFailed", nil) - r.Reporter.Errorf("Error validating AWS credentials: %v", err) - os.Exit(1) + return fmt.Errorf("error validating AWS credentials: %v", err) } if !ok { r.OCMClient.LogEvent("ROSAInitCredentialsInvalid", nil) - r.Reporter.Errorf("AWS credentials are invalid") - os.Exit(1) + return fmt.Errorf("AWS credentials are invalid") } if r.Reporter.IsTerminal() { r.Reporter.Infof("AWS credentials are valid!") } - // Validate AWS quota - // Call `verify quota` as part of init - quota.Cmd.Run(cmd, argv) - // Verify version of `oc` - oc.Cmd.Run(cmd, argv) - // Determine if interactive mode is needed if !interactive.Enabled() && (!cmd.Flags().Changed("mode")) { interactive.Enable() @@ -300,8 +279,7 @@ func run(cmd *cobra.Command, argv []string) { channelGroup := args.channelGroup policyVersion, err := r.OCMClient.GetPolicyVersion(version, channelGroup) if err != nil { - r.Reporter.Errorf("Error getting version: %s", err) - os.Exit(1) + return fmt.Errorf("error getting version: %s", err) } r.Reporter.Debugf("Creating account roles compatible with OpenShift versions up to %s", policyVersion) @@ -319,21 +297,17 @@ func run(cmd *cobra.Command, argv []string) { }, }) if err != nil { - r.Reporter.Errorf("Expected a valid role prefix: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid role prefix: %s", err) } } if len(prefix) > 32 { - r.Reporter.Errorf("Expected a prefix with no more than 32 characters") - os.Exit(1) + return fmt.Errorf("expected a prefix with no more than 32 characters") } if !aws.RoleNameRE.MatchString(prefix) { - r.Reporter.Errorf("Expected a valid role prefix matching %s", aws.RoleNameRE.String()) - os.Exit(1) + return fmt.Errorf("expected a valid role prefix matching %s", aws.RoleNameRE.String()) } if !args.hostedCP && strings.HasSuffix(prefix, "-HCP") { - r.Reporter.Errorf("The '-HCP' suffix is reserved for hosted CP managed policies") - os.Exit(1) + return fmt.Errorf("the '-HCP' suffix is reserved for hosted CP managed policies") } permissionsBoundary := args.permissionsBoundary @@ -347,16 +321,14 @@ func run(cmd *cobra.Command, argv []string) { }, }) if err != nil { - r.Reporter.Errorf("Expected a valid policy ARN for permissions boundary: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid policy ARN for permissions boundary: %s", err) } } if permissionsBoundary != "" { err = aws.ARNValidator(permissionsBoundary) if err != nil { - r.Reporter.Errorf("Expected a valid policy ARN for permissions boundary: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid policy ARN for permissions boundary: %s", err) } } @@ -371,15 +343,13 @@ func run(cmd *cobra.Command, argv []string) { }, }) if err != nil { - r.Reporter.Errorf("Expected a valid path: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid path: %s", err) } } if path != "" && !aws.ARNPath.MatchString(path) { - r.Reporter.Errorf("The specified value for path is invalid. " + - "It must begin and end with '/' and contain only alphanumeric characters and/or '/' characters.") - os.Exit(1) + return fmt.Errorf("the specified value for path is invalid, " + + "it must begin and end with '/' and contain only alphanumeric characters and/or '/' characters") } if interactive.Enabled() && !cmd.Flags().Changed("external-id") { @@ -393,33 +363,28 @@ func run(cmd *cobra.Command, argv []string) { }, }) if err != nil { - r.Reporter.Errorf("Expected a valid STS external ID: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid STS external ID: %s", err) } } if err := validateAccountRolesSTSExternalID(args.externalID); err != nil { - r.Reporter.Errorf("Expected a valid STS external ID: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid STS external ID: %s", err) } if interactive.Enabled() { mode, err = interactive.GetOptionMode(cmd, mode, "Role creation mode") if err != nil { - r.Reporter.Errorf("Expected a valid role creation mode: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid role creation mode: %s", err) } } if args.forcePolicyCreation && mode != interactive.ModeAuto { - r.Reporter.Warnf("Forcing creation of policies only works in auto mode") - os.Exit(1) + return fmt.Errorf("forcing creation of policies only works in auto mode") } policies, err := r.OCMClient.GetPolicies("AccountRole") if err != nil { - r.Reporter.Errorf("Expected a valid role creation mode: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid role creation mode: %s", err) } createClassic := args.classic @@ -431,8 +396,7 @@ func run(cmd *cobra.Command, argv []string) { Required: false, }) if err != nil { - r.Reporter.Errorf("Expected a valid value: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid value: %s", err) } isClassicValueSet = true } @@ -447,8 +411,7 @@ func run(cmd *cobra.Command, argv []string) { Required: false, }) if err != nil { - r.Reporter.Errorf("Expected a valid value: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid value: %s", err) } isHostedCPValueSet = true } @@ -462,8 +425,7 @@ func run(cmd *cobra.Command, argv []string) { Required: createHostedCP, }) if err != nil { - r.Reporter.Errorf("Expected a valid value: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid value: %s", err) } if !isHcpSharedVpc { @@ -483,8 +445,7 @@ func run(cmd *cobra.Command, argv []string) { }, }) if err != nil { - r.Reporter.Errorf("Expected a valid value: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid value: %s", err) } } if interactive.Enabled() && isHcpSharedVpc && !r.Creator.IsGovcloud && createHostedCP { @@ -498,8 +459,7 @@ func run(cmd *cobra.Command, argv []string) { }, }) if err != nil { - r.Reporter.Errorf("Expected a valid value: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid value: %s", err) } } @@ -510,8 +470,7 @@ func run(cmd *cobra.Command, argv []string) { isHcpSharedVpc, err = roles.ValidateSharedVpcInputs(args.vpcEndpointRoleArn, args.route53RoleArn, vpcEndpointRoleArnFlag, route53RoleArnFlag) if err != nil { - r.Reporter.Errorf("%s", err) - os.Exit(1) + return err } } @@ -519,20 +478,18 @@ func run(cmd *cobra.Command, argv []string) { isHcpSharedVpc, err = roles.ValidateSharedVpcInputs(args.vpcEndpointRoleArn, args.route53RoleArn, vpcEndpointRoleArnFlag, route53RoleArnFlag) if err != nil { - r.Reporter.Errorf("%s", err) - os.Exit(1) + return err } } rolesCreator, createRoles := initCreator(r, managedPolicies, createClassic, createHostedCP, isClassicValueSet, isHostedCPValueSet) if !createRoles { - os.Exit(1) + return fmt.Errorf("failed to initialize account role creator") } if fedramp.Enabled() && isHcpSharedVpc { - _ = r.Reporter.Errorf("HCP shared VPC not supported while using a govcloud region") - os.Exit(1) + return fmt.Errorf("HCP shared VPC not supported while using a govcloud region") } input := buildRolesCreationInput(prefix, permissionsBoundary, r.Creator.AccountID, env, policies, @@ -542,19 +499,18 @@ func run(cmd *cobra.Command, argv []string) { case interactive.ModeAuto: err = rolesCreator.createRoles(r, input) if err != nil { - r.Reporter.Errorf("There was an error creating the account roles: %s", err) if strings.Contains(err.Error(), "Throttling") { r.OCMClient.LogEvent("ROSACreateAccountRolesModeAuto", map[string]string{ ocm.Response: ocm.Failure, ocm.Version: policyVersion, ocm.IsThrottle: "true", }) - os.Exit(1) + return fmt.Errorf("there was an error creating the account roles: %s", err) } r.OCMClient.LogEvent("ROSACreateAccountRolesModeAuto", map[string]string{ ocm.Response: ocm.Failure, }) - os.Exit(1) + return fmt.Errorf("there was an error creating the account roles: %s", err) } r.OCMClient.LogEvent("ROSACreateAccountRolesModeAuto", map[string]string{ ocm.Response: ocm.Success, @@ -564,16 +520,14 @@ func run(cmd *cobra.Command, argv []string) { err = aws.GenerateAccountRolePolicyFiles(r.Reporter, env, policies, rolesCreator.skipPermissionFiles(), rolesCreator.getAccountRolesMap(), r.Creator.Partition, args.externalID) if err != nil { - r.Reporter.Errorf("There was an error generating the policy files: %s", err) r.OCMClient.LogEvent("ROSACreateAccountRolesModeManual", map[string]string{ ocm.Response: ocm.Failure, }) - os.Exit(1) + return fmt.Errorf("there was an error generating the policy files: %s", err) } err = rolesCreator.printCommands(r, input) if err != nil { - r.Reporter.Errorf("%s", err) - os.Exit(1) + return err } if r.Reporter.IsTerminal() { r.Reporter.Infof("All policy files saved to the current directory") @@ -582,9 +536,10 @@ func run(cmd *cobra.Command, argv []string) { ocm.Version: policyVersion, }) default: - r.Reporter.Errorf("Invalid mode. Allowed values are %s", interactive.Modes) - os.Exit(1) + return fmt.Errorf("invalid mode. Allowed values are %s", interactive.Modes) } + + return nil } func validateAccountRolesSTSExternalID(externalID string) error { diff --git a/cmd/create/accountroles/cmd_test.go b/cmd/create/accountroles/cmd_test.go index 6c68673313..03c3ff864e 100644 --- a/cmd/create/accountroles/cmd_test.go +++ b/cmd/create/accountroles/cmd_test.go @@ -1,8 +1,27 @@ package accountroles import ( - . "github.com/onsi/ginkgo/v2/dsl/core" + "net/http" + "os" + "path/filepath" + "strings" + + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + . "github.com/openshift-online/ocm-sdk-go/testing" + + awsClient "github.com/openshift/rosa/pkg/aws" + "github.com/openshift/rosa/pkg/interactive" + "github.com/openshift/rosa/pkg/test" +) + +const ( + // Empty versions response: no versions available + versionsResponse = `{"kind":"VersionList","page":1,"size":0,"total":0,"items":[]}` + // Valid versions response with a single version + validVersionsResponse = `{"kind":"VersionList","page":1,"size":1,"total":1,"items":[` + + `{"kind":"Version","id":"openshift-v4.14.0","raw_id":"4.14.0",` + + `"enabled":true,"default":true,"rosa_enabled":true,"channel_group":"stable"}]}` ) const cmdTestExternalID = "223B9588-36A5-ECA4-BE8D-7C673B77CEC1" @@ -23,3 +42,164 @@ var _ = Describe("validateAccountRolesSTSExternalID", func() { Expect(err).To(HaveOccurred(), "invalid external-id should fail validation") }) }) + +var _ = Describe("runWithRuntime", func() { + var t *test.TestingRuntime + + BeforeEach(func() { + t = test.NewTestRuntime() + args = struct { + prefix string + permissionsBoundary string + path string + version string + channelGroup string + managed bool + forcePolicyCreation bool + hostedCP bool + classic bool + route53RoleArn string + vpcEndpointRoleArn string + externalID string + }{} + interactive.SetEnabled(false) + interactive.SetModeKey("") + }) + + It("returns error when GetMode fails", func() { + interactive.SetModeKey("invalid-mode") + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Invalid mode")) + }) + + It("returns error when GetEnv fails", func() { + tmpDir := GinkgoT().TempDir() + configFile := filepath.Join(tmpDir, "ocm.json") + Expect(os.WriteFile(configFile, []byte("{invalid-json"), 0600)).To(Succeed()) + GinkgoT().Setenv("OCM_CONFIG", configFile) + + args.classic = true + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to determine OCM environment")) + }) + + Context("with valid env", func() { + BeforeEach(func() { + tmpDir := GinkgoT().TempDir() + configFile := filepath.Join(tmpDir, "ocm.json") + cfg := `{"url":"https://api.stage.openshift.com","access_token":"test","client_id":"test","token_url":"https://sso.test"}` + Expect(os.WriteFile(configFile, []byte(cfg), 0600)).To(Succeed()) + GinkgoT().Setenv("OCM_CONFIG", configFile) + }) + + It("returns error when force-policy-creation is used with managed policies", func() { + args.classic = true + args.managed = true + args.forcePolicyCreation = true + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("forcing creation of policies only works for unmanaged policies")) + }) + + It("returns error when AWS credentials validation fails", func() { + args.classic = true + + mockAWS := t.RosaRuntime.AWSClient.(*awsClient.MockClient) + mockAWS.EXPECT().ValidateCredentials().Return(false, nil) + + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("AWS credentials are invalid")) + }) + + It("returns error when GetPolicyVersion fails", func() { + args.classic = true + + mockAWS := t.RosaRuntime.AWSClient.(*awsClient.MockClient) + mockAWS.EXPECT().ValidateCredentials().Return(true, nil) + + Expect(Cmd.Flags().Set("mode", "auto")).To(Succeed()) + DeferCleanup(func() { + interactive.SetModeKey("") + Cmd.Flag("mode").Changed = false + }) + + // GetPolicyVersion -> GetLatestVersion -> GetVersions: return error + t.ApiServer.AppendHandlers( + RespondWithJSON(http.StatusInternalServerError, `{"kind":"Error","id":"500","reason":"versions unavailable"}`), + ) + + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("error getting version")) + }) + + It("returns error when prefix exceeds 32 characters", func() { + args.classic = true + args.prefix = strings.Repeat("a", 33) + + mockAWS := t.RosaRuntime.AWSClient.(*awsClient.MockClient) + mockAWS.EXPECT().ValidateCredentials().Return(true, nil) + + Expect(Cmd.Flags().Set("mode", "auto")).To(Succeed()) + DeferCleanup(func() { + interactive.SetModeKey("") + Cmd.Flag("mode").Changed = false + }) + + t.ApiServer.AppendHandlers( + RespondWithJSON(http.StatusOK, validVersionsResponse), + ) + + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("expected a prefix with no more than 32 characters")) + }) + + It("returns error when prefix has invalid characters", func() { + args.classic = true + args.prefix = "bad prefix!" + + mockAWS := t.RosaRuntime.AWSClient.(*awsClient.MockClient) + mockAWS.EXPECT().ValidateCredentials().Return(true, nil) + + Expect(Cmd.Flags().Set("mode", "auto")).To(Succeed()) + DeferCleanup(func() { + interactive.SetModeKey("") + Cmd.Flag("mode").Changed = false + }) + + t.ApiServer.AppendHandlers( + RespondWithJSON(http.StatusOK, validVersionsResponse), + ) + + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("expected a valid role prefix matching")) + }) + + It("returns error when non-HCP prefix ends with -HCP", func() { + args.classic = true + args.prefix = "MyPrefix-HCP" + + mockAWS := t.RosaRuntime.AWSClient.(*awsClient.MockClient) + mockAWS.EXPECT().ValidateCredentials().Return(true, nil) + + Expect(Cmd.Flags().Set("mode", "auto")).To(Succeed()) + DeferCleanup(func() { + interactive.SetModeKey("") + Cmd.Flag("mode").Changed = false + }) + + t.ApiServer.AppendHandlers( + RespondWithJSON(http.StatusOK, validVersionsResponse), + ) + + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("the '-HCP' suffix is reserved for hosted CP managed policies")) + }) + }) +}) diff --git a/cmd/dlt/accountroles/cmd.go b/cmd/dlt/accountroles/cmd.go index e4313f9759..ed0b6f0caa 100644 --- a/cmd/dlt/accountroles/cmd.go +++ b/cmd/dlt/accountroles/cmd.go @@ -98,7 +98,14 @@ func init() { func run(cmd *cobra.Command, _ []string) { r := rosa.NewRuntime().WithAWS().WithOCM() defer r.Cleanup() + err := runWithRuntime(r, cmd) + if err != nil { + r.Reporter.Errorf("%s", err) + os.Exit(1) + } +} +func runWithRuntime(r *rosa.Runtime, cmd *cobra.Command) error { // Determine if interactive mode is needed (if a prefix is not provided, fallback to interactive mode) if !interactive.Enabled() && !cmd.Flags().Changed("mode") || args.prefix == "" { interactive.Enable() @@ -106,14 +113,12 @@ func run(cmd *cobra.Command, _ []string) { mode, err := interactive.GetMode() if err != nil { - r.Reporter.Errorf("%s", err) - os.Exit(1) + return err } env, err := ocm.GetEnv() if err != nil { - r.Reporter.Errorf("Error getting environment %s", err) - os.Exit(1) + return fmt.Errorf("error getting environment %s", err) } deleteClassic, deleteHostedCP := setDeleteRoles(cmd.Flags().Changed("classic"), @@ -121,8 +126,7 @@ func run(cmd *cobra.Command, _ []string) { clusters, err := r.OCMClient.GetAllClusters(r.Creator) if err != nil { - r.Reporter.Errorf("Error getting clusters %s", err) - os.Exit(1) + return fmt.Errorf("error getting clusters %s", err) } prefix := args.prefix @@ -138,42 +142,38 @@ func run(cmd *cobra.Command, _ []string) { }, }) if err != nil { - r.Reporter.Errorf("Expected a valid role prefix: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid role prefix: %s", err) } } if len(prefix) > 32 { - r.Reporter.Errorf("Expected a prefix with no more than 32 characters") - os.Exit(1) + return fmt.Errorf("expected a prefix with no more than 32 characters") } if !aws.RoleNameRE.MatchString(prefix) { - r.Reporter.Errorf("Expected a valid role prefix matching %s", aws.RoleNameRE.String()) - os.Exit(1) + return fmt.Errorf("expected a valid role prefix matching %s", aws.RoleNameRE.String()) } if interactive.Enabled() { mode, err = interactive.GetOptionMode(cmd, mode, "Account role deletion mode") if err != nil { - r.Reporter.Errorf("Expected a valid Account role deletion mode: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid Account role deletion mode: %s", err) } } if deleteClassic { err = deleteAccountRoles(r, cmd, env, prefix, clusters, mode, false) if err != nil { - r.Reporter.Errorf("%s", err) - os.Exit(1) + return err } } if deleteHostedCP { err = deleteAccountRoles(r, cmd, env, prefix, clusters, mode, true) if err != nil { - r.Reporter.Errorf("%s", err) - os.Exit(1) + return err } } + + return nil } func setDeleteRoles(isClassicFlagSet bool, isHostedCPFlagSet bool) (bool, bool) { diff --git a/cmd/dlt/accountroles/cmd_test.go b/cmd/dlt/accountroles/cmd_test.go index 26fbbd0d8e..a058832edd 100644 --- a/cmd/dlt/accountroles/cmd_test.go +++ b/cmd/dlt/accountroles/cmd_test.go @@ -1,8 +1,20 @@ package accountroles import ( + "net/http" + "os" + "path/filepath" + + "go.uber.org/mock/gomock" + + "github.com/aws/aws-sdk-go-v2/service/iam" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + . "github.com/openshift-online/ocm-sdk-go/testing" + + awsClient "github.com/openshift/rosa/pkg/aws" + "github.com/openshift/rosa/pkg/interactive" + "github.com/openshift/rosa/pkg/test" ) var _ = Describe("Delete account roles", func() { @@ -27,3 +39,120 @@ var _ = Describe("Delete account roles", func() { Expect(deleteHostedCP).To(Equal(true)) }) }) + +var _ = Describe("runWithRuntime", func() { + var t *test.TestingRuntime + + BeforeEach(func() { + t = test.NewTestRuntime() + args = struct { + prefix string + hostedCP bool + classic bool + deleteHcpSharedVpcPolicies bool + }{} + interactive.SetEnabled(false) + interactive.SetModeKey("") + }) + + It("returns error when GetMode fails", func() { + interactive.SetModeKey("invalid-mode") + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Invalid mode")) + }) + + Context("with mode set to manual and valid env", func() { + BeforeEach(func() { + Expect(Cmd.Flags().Set("mode", "manual")).To(Succeed()) + setTestOCMConfig(GinkgoT()) + }) + + AfterEach(func() { + interactive.SetModeKey("") + Cmd.Flag("mode").Changed = false + }) + + It("returns error when prefix exceeds 32 characters", func() { + args.prefix = "a]b]c]d]e]f]g]h]i]j]k]l]m]n]o]p]q" + // GetAllClusters + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, + `{"kind":"ClusterList","items":[],"total":0}`)) + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("expected a prefix with no more than 32 characters")) + }) + + It("returns error when prefix has invalid characters", func() { + args.prefix = "bad prefix!" + // GetAllClusters + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, + `{"kind":"ClusterList","items":[],"total":0}`)) + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("expected a valid role prefix matching")) + }) + + It("warns when no classic roles found for prefix", func() { + args.prefix = "MyPrefix" + args.classic = true + Cmd.Flag("classic").Changed = true + DeferCleanup(func() { Cmd.Flag("classic").Changed = false }) + + // GetAllClusters + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, + `{"kind":"ClusterList","items":[],"total":0}`)) + + mockAWS := t.RosaRuntime.AWSClient.(*awsClient.MockClient) + mockAWS.EXPECT().GetAccountRoleForCurrentEnvWithPrefix( + gomock.Any(), "MyPrefix", gomock.Any(), + ).Return([]awsClient.Role{}, nil) + + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).NotTo(HaveOccurred()) + }) + }) +}) + +var _ = Describe("buildCommand", func() { + It("generates detach and delete commands for unmanaged policies", func() { + roleNames := []string{"MyPrefix-Installer-Role"} + policyMap := map[string][]awsClient.PolicyDetail{ + "MyPrefix-Installer-Role": { + {PolicyType: awsClient.Attached, PolicyArn: "arn:aws:iam::123:policy/installer-policy"}, + }, + } + arbitraryPolicyMap := map[string][]awsClient.PolicyDetail{} + policiesOutput := []*iam.GetPolicyOutput{} + + result := buildCommand(roleNames, policyMap, arbitraryPolicyMap, false, policiesOutput) + Expect(result).To(ContainSubstring("detach-role-policy")) + Expect(result).To(ContainSubstring("delete-policy")) + Expect(result).To(ContainSubstring("delete-role")) + Expect(result).To(ContainSubstring("MyPrefix-Installer-Role")) + }) + + It("skips delete-policy for managed policies", func() { + roleNames := []string{"MyPrefix-Installer-Role"} + policyMap := map[string][]awsClient.PolicyDetail{ + "MyPrefix-Installer-Role": { + {PolicyType: awsClient.Attached, PolicyArn: "arn:aws:iam::123:policy/installer-policy"}, + }, + } + arbitraryPolicyMap := map[string][]awsClient.PolicyDetail{} + policiesOutput := []*iam.GetPolicyOutput{} + + result := buildCommand(roleNames, policyMap, arbitraryPolicyMap, true, policiesOutput) + Expect(result).To(ContainSubstring("detach-role-policy")) + Expect(result).NotTo(ContainSubstring("delete-policy")) + Expect(result).To(ContainSubstring("delete-role")) + }) +}) + +func setTestOCMConfig(t GinkgoTInterface) { + tmpDir := t.TempDir() + configFile := filepath.Join(tmpDir, "ocm.json") + cfg := `{"url":"https://api.stage.openshift.com","access_token":"test","client_id":"test","token_url":"https://sso.test"}` + Expect(os.WriteFile(configFile, []byte(cfg), 0600)).To(Succeed()) + t.Setenv("OCM_CONFIG", configFile) +} diff --git a/cmd/dlt/operatorrole/cmd.go b/cmd/dlt/operatorrole/cmd.go index 03bb45fd52..9f4927878f 100644 --- a/cmd/dlt/operatorrole/cmd.go +++ b/cmd/dlt/operatorrole/cmd.go @@ -88,12 +88,18 @@ const ( func run(cmd *cobra.Command, _ []string) { r := rosa.NewRuntime().WithAWS().WithOCM() defer r.Cleanup() - - mode, err := interactive.GetMode() + err := runWithRuntime(r, cmd) if err != nil { r.Reporter.Errorf("%s", err) os.Exit(1) } +} + +func runWithRuntime(r *rosa.Runtime, cmd *cobra.Command) error { + mode, err := interactive.GetMode() + if err != nil { + return err + } // Determine if interactive mode is needed if !interactive.Enabled() && !cmd.Flags().Changed("mode") { @@ -101,15 +107,13 @@ func run(cmd *cobra.Command, _ []string) { } if !cmd.Flag("cluster").Changed && !cmd.Flag(PrefixFlag).Changed { - r.Reporter.Errorf("Either a cluster key or a prefix must be specified.") - os.Exit(1) + return fmt.Errorf("either a cluster key or a prefix must be specified") } if interactive.Enabled() { mode, err = interactive.GetOptionMode(cmd, mode, "Operator roles deletion mode") if err != nil { - r.Reporter.Errorf("Expected a valid operator role deletion mode: %s", err) - os.Exit(1) + return fmt.Errorf("expected a valid operator role deletion mode: %s", err) } } @@ -126,12 +130,10 @@ func run(cmd *cobra.Command, _ []string) { sub, err := r.OCMClient.GetClusterUsingSubscription(clusterKey, r.Creator) if err != nil { if errors.GetType(err) == errors.Conflict { - r.Reporter.Errorf("More than one cluster found with the same name '%s'. Please "+ + return fmt.Errorf("more than one cluster found with the same name '%s', please "+ "use cluster ID instead", clusterKey) - os.Exit(1) } - r.Reporter.Errorf("Error validating cluster '%s': %v", clusterKey, err) - os.Exit(1) + return fmt.Errorf("error validating cluster '%s': %v", clusterKey, err) } if sub != nil { clusterKey = sub.ClusterID() @@ -139,18 +141,15 @@ func run(cmd *cobra.Command, _ []string) { cluster, err := r.OCMClient.GetCluster(clusterKey, r.Creator) if err != nil { if errors.GetType(err) != errors.NotFound { - r.Reporter.Errorf("Error validating cluster '%s': %v", clusterKey, err) - os.Exit(1) + return fmt.Errorf("error validating cluster '%s': %v", clusterKey, err) } else if sub == nil { - r.Reporter.Errorf("Failed to get cluster '%s': %v", r.ClusterKey, err) - os.Exit(1) + return fmt.Errorf("failed to get cluster '%s': %v", r.ClusterKey, err) } } if cluster != nil && cluster.ID() != "" { - r.Reporter.Errorf("Cluster '%s' is in '%s' state. Operator roles can be deleted only for the "+ + return fmt.Errorf("cluster '%s' is in '%s' state, operator roles can be deleted only for the "+ "uninstalled clusters", cluster.ID(), cluster.State()) - os.Exit(1) } isHypershift := false if cluster != nil { @@ -166,8 +165,7 @@ func run(cmd *cobra.Command, _ []string) { } credRequests, err := r.OCMClient.GetCredRequests(isHypershift) if err != nil { - r.Reporter.Errorf("Error getting operator credential request from OCM %s", err) - os.Exit(1) + return fmt.Errorf("error getting operator credential request from OCM %s", err) } foundOperatorRoles, _ = r.AWSClient.GetOperatorRolesFromAccountByClusterID(sub.ClusterID(), credRequests) } else { @@ -178,26 +176,23 @@ func run(cmd *cobra.Command, _ []string) { } hasClusterUsingOperatorRolesPrefix, err := r.OCMClient.HasAClusterUsingOperatorRolesPrefix(args.prefix) if err != nil { - r.Reporter.Errorf("There was a problem checking if any clusters"+ + return fmt.Errorf("there was a problem checking if any clusters"+ " are using Operator Roles Prefix '%s' : %v", args.prefix, err) - os.Exit(1) } if hasClusterUsingOperatorRolesPrefix { if spin != nil { spin.Stop() } - r.Reporter.Errorf("There are clusters using Operator Roles Prefix '%s', can't delete the IAM roles", args.prefix) - os.Exit(1) + return fmt.Errorf("there are clusters using Operator Roles Prefix '%s', "+ + "can't delete the IAM roles", args.prefix) } credRequests, err := r.OCMClient.GetAllCredRequests() if err != nil { - r.Reporter.Errorf("Error getting operator credential request from OCM %v", err) - os.Exit(1) + return fmt.Errorf("error getting operator credential request from OCM %v", err) } foundOperatorRoles, err = r.AWSClient.GetOperatorRolesFromAccountByPrefix(args.prefix, credRequests) if err != nil { - r.Reporter.Errorf("There was a problem retrieving the Operator Roles from AWS: %v", err) - os.Exit(1) + return fmt.Errorf("there was a problem retrieving the Operator Roles from AWS: %v", err) } } @@ -212,7 +207,7 @@ func run(cmd *cobra.Command, _ []string) { noRoleOutput = fmt.Sprintf("%s for cluster '%s'", noRoleOutput, clusterKey) } r.Reporter.Infof("%s", noRoleOutput) - return + return nil } if spin != nil { spin.Stop() @@ -220,13 +215,11 @@ func run(cmd *cobra.Command, _ []string) { _, roleARN, err := r.AWSClient.CheckRoleExists(foundOperatorRoles[0]) if err != nil { - r.Reporter.Errorf("Failed to get '%s' role ARN", foundOperatorRoles[0]) - os.Exit(1) + return fmt.Errorf("failed to get '%s' role ARN", foundOperatorRoles[0]) } managedPolicies, err := r.AWSClient.HasManagedPolicies(roleARN) if err != nil { - r.Reporter.Errorf("Failed to determine if cluster has managed policies: %v", err) - os.Exit(1) + return fmt.Errorf("failed to determine if cluster has managed policies: %v", err) } errOccured := false @@ -280,8 +273,7 @@ func run(cmd *cobra.Command, _ []string) { r.OCMClient.LogEvent("ROSADeleteOperatorroleModeManual", nil) policyMap, arbitraryPolicyMap, err := r.AWSClient.GetOperatorRolePolicies(foundOperatorRoles) if err != nil { - r.Reporter.Errorf("There was an error getting the policy: %v", err) - os.Exit(1) + return fmt.Errorf("there was an error getting the policy: %v", err) } // Get HCP shared vpc policy details if the user is deleting roles related to HCP shared vpc @@ -303,9 +295,10 @@ func run(cmd *cobra.Command, _ []string) { } fmt.Println(commands) default: - r.Reporter.Errorf("Invalid mode. Allowed values are %s", interactive.Modes) - os.Exit(1) + return fmt.Errorf("invalid mode. Allowed values are %s", interactive.Modes) } + + return nil } func buildCommand(r *rosa.Runtime, roleNames []string, policyMap map[string][]string, diff --git a/cmd/dlt/operatorrole/cmd_test.go b/cmd/dlt/operatorrole/cmd_test.go new file mode 100644 index 0000000000..d7343511be --- /dev/null +++ b/cmd/dlt/operatorrole/cmd_test.go @@ -0,0 +1,165 @@ +package operatorrole + +import ( + "net/http" + + "go.uber.org/mock/gomock" + + "github.com/aws/aws-sdk-go-v2/service/iam" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/openshift-online/ocm-sdk-go/testing" + + awsClient "github.com/openshift/rosa/pkg/aws" + "github.com/openshift/rosa/pkg/interactive" + "github.com/openshift/rosa/pkg/test" +) + +var _ = Describe("runWithRuntime", func() { + var t *test.TestingRuntime + + BeforeEach(func() { + t = test.NewTestRuntime() + args = struct { + prefix string + deleteHcpSharedVpcPolicies bool + }{} + interactive.SetEnabled(false) + interactive.SetModeKey("") + }) + + It("returns error when GetMode fails", func() { + interactive.SetModeKey("invalid-mode") + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Invalid mode")) + }) + + It("returns error when neither cluster nor prefix is provided", func() { + interactive.SetModeKey("auto") + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("either a cluster key or a prefix must be specified")) + }) + + Context("with prefix set", func() { + BeforeEach(func() { + Expect(Cmd.Flags().Set("mode", "manual")).To(Succeed()) + args.prefix = "test-prefix" + Cmd.Flag(PrefixFlag).Changed = true + }) + + AfterEach(func() { + interactive.SetModeKey("") + Cmd.Flag("mode").Changed = false + Cmd.Flag(PrefixFlag).Changed = false + }) + + It("reports no operator roles when none found for prefix", func() { + // HasAClusterUsingOperatorRolesPrefix -> clusters list -> empty + t.ApiServer.AppendHandlers( + RespondWithJSON(http.StatusOK, `{"kind":"ClusterList","items":[],"total":0}`), + ) + // GetAllCredRequests -> GetCredRequests(false) classic + t.ApiServer.AppendHandlers( + RespondWithJSON(http.StatusOK, `{"kind":"STSOperatorList","items":[],"total":0}`), + ) + // GetAllCredRequests -> GetCredRequests(true) hcp + t.ApiServer.AppendHandlers( + RespondWithJSON(http.StatusOK, `{"kind":"STSOperatorList","items":[],"total":0}`), + ) + + mockAWS := t.RosaRuntime.AWSClient.(*awsClient.MockClient) + mockAWS.EXPECT().GetOperatorRolesFromAccountByPrefix( + "test-prefix", gomock.Any(), + ).Return([]string{}, nil) + + err := t.StdOutReader.Record() + Expect(err).NotTo(HaveOccurred()) + + err = runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).NotTo(HaveOccurred()) + + stdout, err := t.StdOutReader.Read() + Expect(err).NotTo(HaveOccurred()) + Expect(stdout).To(ContainSubstring("no operator roles to delete")) + }) + + It("returns error when a cluster is using the prefix", func() { + // HasAClusterUsingOperatorRolesPrefix -> clusters list -> found one + t.ApiServer.AppendHandlers( + RespondWithJSON(http.StatusOK, `{"kind":"ClusterList","items":[{"id":"abc"}],"total":1}`), + ) + + err := runWithRuntime(t.RosaRuntime, Cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("there are clusters using Operator Roles Prefix")) + }) + }) +}) + +var _ = Describe("buildCommand", func() { + var t *test.TestingRuntime + + BeforeEach(func() { + t = test.NewTestRuntime() + }) + + It("generates detach, delete-version, delete-policy, and delete-role commands", func() { + roleNames := []string{"my-operator-role"} + policyMap := map[string][]string{ + "my-operator-role": {"arn:aws:iam::123:policy/op-policy"}, + } + arbitraryPolicyMap := map[string][]string{} + policiesOutput := []*iam.GetPolicyOutput{} + + mockAWS := t.RosaRuntime.AWSClient.(*awsClient.MockClient) + mockAWS.EXPECT().ListPolicyVersions("arn:aws:iam::123:policy/op-policy").Return( + []awsClient.PolicyVersion{ + {VersionID: "v1", IsDefaultVersion: true}, + {VersionID: "v2", IsDefaultVersion: false}, + }, nil, + ) + + result := buildCommand(t.RosaRuntime, roleNames, policyMap, arbitraryPolicyMap, false, policiesOutput) + Expect(result).To(ContainSubstring("detach-role-policy")) + Expect(result).To(ContainSubstring("delete-policy-version")) + Expect(result).To(ContainSubstring("v2")) + Expect(result).To(ContainSubstring("delete-policy")) + Expect(result).To(ContainSubstring("delete-role")) + Expect(result).To(ContainSubstring("my-operator-role")) + }) + + It("uses shared-vpc policy ARN for managed policies", func() { + roleNames := []string{"my-operator-role"} + policyMap := map[string][]string{ + "my-operator-role": {"arn:aws:iam::123:policy/op-policy"}, + } + arbitraryPolicyMap := map[string][]string{} + policiesOutput := []*iam.GetPolicyOutput{} + + mockAWS := t.RosaRuntime.AWSClient.(*awsClient.MockClient) + mockAWS.EXPECT().ListPolicyVersions("arn:aws:iam::123:policy/op-policy").Return( + []awsClient.PolicyVersion{}, nil, + ) + + result := buildCommand(t.RosaRuntime, roleNames, policyMap, arbitraryPolicyMap, true, policiesOutput) + Expect(result).To(ContainSubstring("detach-role-policy")) + Expect(result).To(ContainSubstring("delete-role")) + Expect(result).To(ContainSubstring(awsClient.SharedVpcAssumeRolePrefix)) + }) + + It("detaches arbitrary policies", func() { + roleNames := []string{"my-operator-role"} + policyMap := map[string][]string{} + arbitraryPolicyMap := map[string][]string{ + "my-operator-role": {"arn:aws:iam::123:policy/extra-policy"}, + } + policiesOutput := []*iam.GetPolicyOutput{} + + result := buildCommand(t.RosaRuntime, roleNames, policyMap, arbitraryPolicyMap, false, policiesOutput) + Expect(result).To(ContainSubstring("detach-role-policy")) + Expect(result).To(ContainSubstring("extra-policy")) + Expect(result).To(ContainSubstring("delete-role")) + }) +}) diff --git a/cmd/dlt/operatorrole/operatorrole_suite_test.go b/cmd/dlt/operatorrole/operatorrole_suite_test.go new file mode 100644 index 0000000000..dc8e6347fb --- /dev/null +++ b/cmd/dlt/operatorrole/operatorrole_suite_test.go @@ -0,0 +1,13 @@ +package operatorrole + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestDeleteOperatorRole(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Delete operator-roles suite") +}