diff --git a/cmd/create/accountroles/cmd.go b/cmd/create/accountroles/cmd.go index 858326342c..48006e0985 100644 --- a/cmd/create/accountroles/cmd.go +++ b/cmd/create/accountroles/cmd.go @@ -171,10 +171,6 @@ func run(cmd *cobra.Command, argv []string) { os.Exit(1) } - if !cmd.Flag("hosted-cp").Changed { - rosa.HostedClusterOnlyFlag(r, cmd, route53RoleArnFlag) - rosa.HostedClusterOnlyFlag(r, cmd, vpcEndpointRoleArnFlag) - } isHcpSharedVpc, err := validateSharedVpcInputs(args.hostedCP, args.vpcEndpointRoleArn, args.route53RoleArn) if err != nil { r.Reporter.Errorf("%s", err) @@ -409,6 +405,11 @@ func run(cmd *cobra.Command, argv []string) { isHostedCPValueSet = true } + if !createHostedCP { + rosa.HostedClusterOnlyFlag(r, cmd, route53RoleArnFlag) + rosa.HostedClusterOnlyFlag(r, cmd, vpcEndpointRoleArnFlag) + } + rolesCreator, createRoles := initCreator(r, managedPolicies, createClassic, createHostedCP, isClassicValueSet, isHostedCPValueSet) if !createRoles { diff --git a/cmd/create/accountroles/creators.go b/cmd/create/accountroles/creators.go index 9f878601d9..2270c1e387 100644 --- a/cmd/create/accountroles/creators.go +++ b/cmd/create/accountroles/creators.go @@ -201,7 +201,7 @@ func (up *unmanagedPoliciesCreator) printCommands(r *rosa.Runtime, input *accoun createPolicy := buildCreatePolicyCommand(policyName, policyDocument, iamTags, input.path) - policyARN := aws.GetPolicyARN(r.Creator.Partition, input.accountID, accRoleName, input.path) + policyARN := aws.GetPolicyArnWithSuffix(r.Creator.Partition, input.accountID, accRoleName, input.path) attachRolePolicy := buildAttachRolePolicyCommand(accRoleName, policyARN) @@ -279,7 +279,7 @@ func createRoleUnmanagedPolicy(r *rosa.Runtime, input *accountRolesCreationInput policyPermissionDetail := aws.GetPolicyDetails(input.policies, filename) - policyARN := aws.GetPolicyARN(r.Creator.Partition, r.Creator.AccountID, accRoleName, input.path) + policyARN := aws.GetPolicyArnWithSuffix(r.Creator.Partition, r.Creator.AccountID, accRoleName, input.path) r.Reporter.Debugf("Creating permission policy '%s'", policyARN) if args.forcePolicyCreation { @@ -467,9 +467,9 @@ func attachHcpSharedVpcPolicy(r *rosa.Runtime, sharedVpcRoleArn string, roleName return err } policyName := fmt.Sprintf(aws.AssumeRolePolicyPrefix, userProvidedRoleName) - policyArn := aws.GetPolicyARN(r.Creator.Partition, r.Creator.AccountID, roleName, path) - policyArn, err = r.AWSClient.EnsurePolicyWithName(policyArn, policyDetails, - defaultPolicyVersion, policyTags, path, policyName) + policyArn := aws.GetPolicyArn(r.Creator.Partition, r.Creator.AccountID, policyName, path) + policyArn, err = r.AWSClient.EnsurePolicy(policyArn, policyDetails, + defaultPolicyVersion, policyTags, path) if err != nil { return err } diff --git a/cmd/create/accountroles/creators_test.go b/cmd/create/accountroles/creators_test.go index 4c140fae61..b38d8e9a17 100644 --- a/cmd/create/accountroles/creators_test.go +++ b/cmd/create/accountroles/creators_test.go @@ -85,8 +85,8 @@ var _ = Describe("Accountroles", Ordered, func() { mockClient.EXPECT().AttachRolePolicy(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(6) mockClient.EXPECT().EnsureRole(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return("arn::role:role-123", nil).AnyTimes() - mockClient.EXPECT().EnsurePolicyWithName(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), - gomock.Any(), gomock.Any()).Return("arn::policy:123", nil).Times(2) + mockClient.EXPECT().EnsurePolicy(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), + gomock.Any()).Return("arn::policy:123", nil).Times(2) r := rosa.NewRuntime() r.AWSClient = mockClient diff --git a/cmd/create/ocmrole/cmd.go b/cmd/create/ocmrole/cmd.go index 82f8168fb0..97bc821571 100644 --- a/cmd/create/ocmrole/cmd.go +++ b/cmd/create/ocmrole/cmd.go @@ -376,7 +376,7 @@ func buildCommands(prefix string, roleName string, rolePath string, permissionsB return "", err } } else { - policyARN = aws.GetPolicyARN(creator.Partition, creator.AccountID, roleName, rolePath) + policyARN = aws.GetPolicyArnWithSuffix(creator.Partition, creator.AccountID, roleName, rolePath) } attachRolePolicy := awscb.NewIAMCommandBuilder(). SetCommand(awscb.AttachRolePolicy). @@ -448,7 +448,7 @@ func createRoles(r *rosa.Runtime, prefix string, roleName string, rolePath strin return "", err } } else { - policyARN = aws.GetPolicyARN(r.Creator.Partition, r.Creator.AccountID, roleName, rolePath) + policyARN = aws.GetPolicyArnWithSuffix(r.Creator.Partition, r.Creator.AccountID, roleName, rolePath) } if !confirm.Prompt(true, "Create the '%s' role?", roleName) { os.Exit(0) diff --git a/cmd/create/operatorroles/by_prefix.go b/cmd/create/operatorroles/by_prefix.go index b0054ce3c9..7ba9fe66fc 100644 --- a/cmd/create/operatorroles/by_prefix.go +++ b/cmd/create/operatorroles/by_prefix.go @@ -304,6 +304,7 @@ func createRolesByPrefix(r *rosa.Runtime, prefix string, permissionsBoundary str } var policyArn string + var policyArns []string filename := aws.GetOperatorPolicyKey(credrequest, hostedCPPolicies, isSharedVpc) if managedPolicies { policyArn, err = aws.GetManagedPolicyARN(policies, filename) @@ -312,16 +313,19 @@ func createRolesByPrefix(r *rosa.Runtime, prefix string, permissionsBoundary str } if isSharedVpc { if credrequest == aws.IngressOperatorCloudCredentialsRoleType { - policyArn, err = getHcpSharedVpcPolicy(r, sharedVpcRoleArn, roleName, operator, - path, defaultPolicyVersion) + sharedVpcPolicyArn, err := getHcpSharedVpcPolicy(r, sharedVpcRoleArn, roleName, defaultPolicyVersion) if err != nil { return err } + policyArns = append(policyArns, sharedVpcPolicyArn) } else if credrequest == aws.ControlPlaneCloudCredentialsRoleType { - policyArn, err = getHcpSharedVpcPolicy(r, sharedVpcEndpointRoleArn, roleName, - operator, path, defaultPolicyVersion) - if err != nil { - return err + for _, arn := range []string{sharedVpcEndpointRoleArn, sharedVpcRoleArn} { + sharedVpcPolicyArn, err := getHcpSharedVpcPolicy(r, arn, path, + defaultPolicyVersion) + if err != nil { + return err + } + policyArns = append(policyArns, sharedVpcPolicyArn) } } } @@ -363,6 +367,7 @@ func createRolesByPrefix(r *rosa.Runtime, prefix string, permissionsBoundary str } } } + policyArns = append(policyArns, policyArn) policyDetails := aws.GetPolicyDetails(policies, "operator_iam_role_policy") policy, err := aws.GenerateOperatorRolePolicyDocByOidcEndpointUrl(r.Creator.Partition, oidcEndpointUrl, @@ -393,10 +398,12 @@ func createRolesByPrefix(r *rosa.Runtime, prefix string, permissionsBoundary str r.Reporter.Infof("Created role '%s' with ARN '%s'", roleName, roleARN) } - r.Reporter.Debugf("Attaching permission policy '%s' to role '%s'", policyArn, roleName) - err = r.AWSClient.AttachRolePolicy(r.Reporter, roleName, policyArn) - if err != nil { - return err + for _, arn := range policyArns { + r.Reporter.Debugf("Attaching permission policy '%s' to role '%s'", arn, roleName) + err = r.AWSClient.AttachRolePolicy(r.Reporter, roleName, arn) + if err != nil { + return err + } } } diff --git a/cmd/create/operatorroles/common_utils.go b/cmd/create/operatorroles/common_utils.go index 5169470ef4..c57489ce18 100644 --- a/cmd/create/operatorroles/common_utils.go +++ b/cmd/create/operatorroles/common_utils.go @@ -4,7 +4,6 @@ import ( "fmt" awsCommonUtils "github.com/openshift-online/ocm-common/pkg/aws/utils" - cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" errors "github.com/zgalor/weberr" "github.com/openshift/rosa/pkg/aws" @@ -26,6 +25,9 @@ const policyDocumentBody = ` \ } }'` +const policyDetails = "{\n \"Version\": \"2012-10-17\",\n \"Statement\": {\n \"Effect\": \"Allow\",\n " + + "\"Action\": \"sts:AssumeRole\",\n \"Resource\": \"%{shared_vpc_role_arn}\"\n }\n}\n" + func computePolicyARN(creator aws.Creator, prefix string, namespace string, name string, path string) string { if prefix == "" { @@ -75,18 +77,19 @@ func validateIngressOperatorPolicyOverride(r *rosa.Runtime, policyArn string, sh return nil } -func getHcpSharedVpcPolicy(r *rosa.Runtime, roleArn string, roleName string, - operator *cmv1.STSOperator, path string, defaultPolicyVersion string) (string, error) { - policyDetails := "{\n \"Version\": \"2012-10-17\",\n \"Statement\": {\n \"Effect\": \"Allow\",\n " + - "\"Action\": \"sts:AssumeRole\",\n \"Resource\": \"%{shared_vpc_role_arn}\"\n }\n}\n" - policyDetails = aws.InterpolatePolicyDocument(r.Creator.Partition, policyDetails, map[string]string{ +func getHcpSharedVpcPolicy(r *rosa.Runtime, roleArn string, path string, defaultPolicyVersion string) (string, error) { + + interpolatedPolicyDetails := aws.InterpolatePolicyDocument(r.Creator.Partition, policyDetails, map[string]string{ "shared_vpc_role_arn": roleArn, }) - policy := aws.GetOperatorPolicyARN(r.Creator.Partition, r.Creator.AccountID, - aws.SharedVpcAssumeRolePrefix+"-"+roleName, operator.Namespace(), operator.Name(), path) + userProvidedRoleName, err := aws.GetResourceIdFromARN(roleArn) + if err != nil { + return "", err + } + policyName := fmt.Sprintf(aws.AssumeRolePolicyPrefix, userProvidedRoleName) + policy := aws.GetPolicyArn(r.Creator.Partition, r.Creator.AccountID, policyName, "") - var err error - policyArn, err := r.AWSClient.EnsurePolicy(policy, policyDetails, defaultPolicyVersion, + policyArn, err := r.AWSClient.EnsurePolicy(policy, interpolatedPolicyDetails, defaultPolicyVersion, map[string]string{tags.RedHatManaged: helper.True}, path) if err != nil { return policyArn, err @@ -96,16 +99,17 @@ func getHcpSharedVpcPolicy(r *rosa.Runtime, roleArn string, roleName string, func getHcpSharedVpcPolicyDetails(r *rosa.Runtime, roleArn string, roleName string, iamTags map[string]string, path string) (string, string) { - policyDetails := aws.InterpolatePolicyDocument(r.Creator.Partition, policyDocumentBody, map[string]string{ - "shared_vpc_role_arn": roleArn, - }) + interpolatedPolicyDetails := aws.InterpolatePolicyDocument(r.Creator.Partition, policyDocumentBody, + map[string]string{ + "shared_vpc_role_arn": roleArn, + }) policyName := aws.SharedVpcAssumeRolePrefix + "-" + roleName createPolicy := awscb.NewIAMCommandBuilder(). SetCommand(awscb.CreatePolicy). AddParam(awscb.PolicyName, policyName). - AddParam(awscb.PolicyDocument, policyDetails). + AddParam(awscb.PolicyDocument, interpolatedPolicyDetails). AddTags(iamTags). AddParam(awscb.Path, path). Build() diff --git a/cmd/create/operatorroles/common_utils_test.go b/cmd/create/operatorroles/common_utils_test.go index 4fc5d6a874..0e6ac8ec67 100644 --- a/cmd/create/operatorroles/common_utils_test.go +++ b/cmd/create/operatorroles/common_utils_test.go @@ -8,7 +8,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" errors "github.com/zgalor/weberr" "github.com/openshift/rosa/pkg/aws" @@ -25,7 +24,6 @@ var _ = Describe("Create dns domain", func() { var testRoleName = "test" var testIamTags = map[string]string{tags.RedHatManaged: aws.TrueString} var testPath = "/path" - var testOperator *cmv1.STSOperator var testVersion = "2012-10-17" var mockClient *aws.MockClient @@ -37,10 +35,6 @@ var _ = Describe("Create dns domain", func() { runtime.AWSClient = mockClient mockClient.EXPECT().GetCreator().Return(&aws.Creator{Partition: testPartition}, nil) - var err error - testOperator, err = cmv1.NewSTSOperator().Namespace("test").Namespace("test-namespace").Build() - Expect(err).ToNot(HaveOccurred()) - creator, err := runtime.AWSClient.GetCreator() Expect(err).ToNot(HaveOccurred()) runtime.Creator = creator @@ -63,14 +57,14 @@ var _ = Describe("Create dns domain", func() { returnedArn := "arn:aws:iam::123123123123:policy/test" mockClient.EXPECT().EnsurePolicy(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(returnedArn, nil) - arn, err := getHcpSharedVpcPolicy(runtime, testArn, testRoleName, testOperator, testPath, testVersion) + arn, err := getHcpSharedVpcPolicy(runtime, testArn, testPath, testVersion) Expect(err).ToNot(HaveOccurred()) Expect(arn).To(Equal(returnedArn)) }) It("KO: Returns empty policy when fails", func() { mockClient.EXPECT().EnsurePolicy(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return("", errors.UserErrorf("Failed")) - arn, err := getHcpSharedVpcPolicy(runtime, testArn, testRoleName, testOperator, testPath, testVersion) + arn, err := getHcpSharedVpcPolicy(runtime, testArn, testPath, testVersion) Expect(err).To(HaveOccurred()) Expect(arn).To(Equal("")) }) diff --git a/cmd/dlt/operatorrole/cmd.go b/cmd/dlt/operatorrole/cmd.go index 2920f5e736..b2e7c2a741 100644 --- a/cmd/dlt/operatorrole/cmd.go +++ b/cmd/dlt/operatorrole/cmd.go @@ -44,7 +44,7 @@ var args struct { var Cmd = &cobra.Command{ Use: "operator-roles", - Aliases: []string{"operatorrole"}, + Aliases: []string{"operatorrole", "operatorroles"}, Short: "Delete Operator Roles", Long: "Cleans up operator roles of deleted STS cluster.", Example: ` # Delete Operator roles for cluster named "mycluster" diff --git a/cmd/upgrade/accountroles/cmd.go b/cmd/upgrade/accountroles/cmd.go index ebc73bd2bb..00a063a782 100644 --- a/cmd/upgrade/accountroles/cmd.go +++ b/cmd/upgrade/accountroles/cmd.go @@ -291,7 +291,7 @@ func upgradeAccountRolePolicies(reporter *rprtr.Object, awsClient aws.Client, pr continue } filename := fmt.Sprintf("sts_%s_permission_policy", file) - policyARN := aws.GetPolicyARN(partition, accountID, roleName, policyPath) + policyARN := aws.GetPolicyArnWithSuffix(partition, accountID, roleName, policyPath) policyDetails := aws.GetPolicyDetails(policies, filename) policyARN, err := awsClient.EnsurePolicy(policyARN, policyDetails, @@ -333,7 +333,7 @@ func buildCommands(prefix string, partition string, accountID string, isUpgradeN if isUpgradeNeedForAccountRolePolicies { for file, role := range aws.AccountRoles { accRoleName := common.GetRoleName(prefix, role.Name) - policyARN := aws.GetPolicyARN(partition, accountID, accRoleName, policyPath) + policyARN := aws.GetPolicyArnWithSuffix(partition, accountID, accRoleName, policyPath) _, err := awsClient.IsPolicyExists(policyARN) policyExists := err == nil policyName := aws.GetPolicyName(accRoleName) diff --git a/cmd/upgrade/roles/cmd.go b/cmd/upgrade/roles/cmd.go index 9edceee50b..ea382c5a64 100644 --- a/cmd/upgrade/roles/cmd.go +++ b/cmd/upgrade/roles/cmd.go @@ -529,7 +529,7 @@ func handleAccountRolePolicyARN( attachedPoliciesDetail := aws.FindAllAttachedPolicyDetails(policiesDetails) - generatedPolicyARN := aws.GetPolicyARN(partition, accountID, roleName, rolePath) + generatedPolicyARN := aws.GetPolicyArnWithSuffix(partition, accountID, roleName, rolePath) if len(attachedPoliciesDetail) == 0 { return generatedPolicyARN, nil } diff --git a/pkg/aws/client.go b/pkg/aws/client.go index 7104035ea7..7c6e77473d 100644 --- a/pkg/aws/client.go +++ b/pkg/aws/client.go @@ -129,8 +129,6 @@ type Client interface { path string) (string, error) EnsurePolicy(policyArn string, document string, version string, tagList map[string]string, path string) (string, error) - EnsurePolicyWithName(policyArn string, document string, version string, tagList map[string]string, - path string, policyName string) (string, error) AttachRolePolicy(reporter *reporter.Object, roleName string, policyARN string) error CreateOpenIDConnectProvider(issuerURL string, thumbprint string, clusterID string) (string, error) DeleteOpenIDConnectProvider(providerURL string) error diff --git a/pkg/aws/client_mock.go b/pkg/aws/client_mock.go index 30c6becb6f..01b5c88b96 100644 --- a/pkg/aws/client_mock.go +++ b/pkg/aws/client_mock.go @@ -376,21 +376,6 @@ func (mr *MockClientMockRecorder) EnsurePolicy(policyArn, document, version, tag return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsurePolicy", reflect.TypeOf((*MockClient)(nil).EnsurePolicy), policyArn, document, version, tagList, path) } -// EnsurePolicyWithName mocks base method. -func (m *MockClient) EnsurePolicyWithName(policyArn, document, version string, tagList map[string]string, path, policyName string) (string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "EnsurePolicyWithName", policyArn, document, version, tagList, path, policyName) - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// EnsurePolicyWithName indicates an expected call of EnsurePolicyWithName. -func (mr *MockClientMockRecorder) EnsurePolicyWithName(policyArn, document, version, tagList, path, policyName any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsurePolicyWithName", reflect.TypeOf((*MockClient)(nil).EnsurePolicyWithName), policyArn, document, version, tagList, path, policyName) -} - // EnsureRole mocks base method. func (m *MockClient) EnsureRole(reporter *reporter.Object, name, policy, permissionsBoundary, version string, tagList map[string]string, path string, managedPolicies bool) (string, error) { m.ctrl.T.Helper() diff --git a/pkg/aws/helpers.go b/pkg/aws/helpers.go index 964184b2fd..2de989672e 100644 --- a/pkg/aws/helpers.go +++ b/pkg/aws/helpers.go @@ -451,10 +451,14 @@ func GetAdminPolicyARN(partition string, accountID string, name string, path str return getPolicyARN(partition, accountID, GetAdminPolicyName(name), path) } -func GetPolicyARN(partition string, accountID string, name string, path string) string { +func GetPolicyArnWithSuffix(partition string, accountID string, name string, path string) string { return getPolicyARN(partition, accountID, GetPolicyName(name), path) } +func GetPolicyArn(partition string, accountID string, name string, path string) string { + return getPolicyARN(partition, accountID, name, path) +} + func getPolicyARN(partition string, accountID string, name string, path string) string { str := fmt.Sprintf("arn:%s:iam::%s:policy", partition, accountID) if path != "" { diff --git a/pkg/aws/policies.go b/pkg/aws/policies.go index 910cc6e226..445b469c96 100644 --- a/pkg/aws/policies.go +++ b/pkg/aws/policies.go @@ -325,11 +325,6 @@ func (c *awsClient) EnsurePolicy(policyArn string, document string, return c.ensurePolicyHelper(policyArn, document, version, tagList, path, false, "") } -func (c *awsClient) EnsurePolicyWithName(policyArn string, document string, - version string, tagList map[string]string, path string, policyName string) (string, error) { - return c.ensurePolicyHelper(policyArn, document, version, tagList, path, false, policyName) -} - func (c *awsClient) ensurePolicyHelper(policyArn string, document string, version string, tagList map[string]string, path string, force bool, policyName string) (string, error) { output, err := c.IsPolicyExists(policyArn) diff --git a/pkg/rosa/helpers.go b/pkg/rosa/helpers.go index 9e006c2457..6f17c5ee6a 100644 --- a/pkg/rosa/helpers.go +++ b/pkg/rosa/helpers.go @@ -6,10 +6,14 @@ import ( "github.com/spf13/cobra" ) +const hostedCpFlagName = "hosted-cp" + func HostedClusterOnlyFlag(r *Runtime, cmd *cobra.Command, flagName string) { - isFlagSet := cmd.Flags().Changed(flagName) - if isFlagSet { - r.Reporter.Errorf("Setting the `%s` flag is only supported for hosted clusters", flagName) - os.Exit(1) + if cmd.Flag(hostedCpFlagName) == nil || (cmd.Flag(hostedCpFlagName) != nil && !cmd.Flag(hostedCpFlagName).Changed) { + isFlagSet := cmd.Flags().Changed(flagName) + if isFlagSet { + r.Reporter.Errorf("Setting the `%s` flag is only supported for hosted clusters", flagName) + os.Exit(1) + } } }