diff --git a/pkg/provider/aws/iam.go b/pkg/provider/aws/iam.go index 6da29c9d..b7a6916c 100644 --- a/pkg/provider/aws/iam.go +++ b/pkg/provider/aws/iam.go @@ -1,31 +1,12 @@ package aws import ( - "encoding/json" "fmt" - "time" - - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/iam" - awsv1alpha1 "github.com/openshift/aws-account-operator/api/v1alpha1" - "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog/v2" ) -type awsStatement struct { - Effect string `json:"Effect"` - Action []string `json:"Action"` - Resource []string `json:"Resource,omitempty"` - Condition *awsv1alpha1.Condition `json:"Condition,omitempty"` - Principal *awsv1alpha1.Principal `json:"Principal,omitempty"` -} - -type policyDoc struct { - Version string `json:"Version"` - Statement []awsStatement `json:"Statement"` -} - func CreateIAMUserAndAttachPolicy(awsClient Client, username, policyArn *string) error { output, err := awsClient.CreateUser(&iam.CreateUserInput{UserName: username}) if err != nil { @@ -77,137 +58,6 @@ func DeleteUserAccessKeys(awsClient Client, username *string) error { return nil } -func RefreshIAMPolicy(awsClient Client, federatedRole *awsv1alpha1.AWSFederatedRole, awsAccountID, uid string) error { - roleName := federatedRole.Name + "-" + uid - policyName := federatedRole.Spec.AWSCustomPolicy.Name + "-" + uid - policyArn := fmt.Sprintf("arn:aws:iam::%s:policy/%s", awsAccountID, policyName) - - statements := make([]awsStatement, 0, len(federatedRole.Spec.AWSCustomPolicy.Statements)) - for _, sm := range federatedRole.Spec.AWSCustomPolicy.Statements { - statement := awsStatement(sm) - statements = append(statements, statement) - } - - pd := policyDoc{ - Version: "2012-10-17", - Statement: statements, - } - - payload, err := json.Marshal(&pd) - if err != nil { - return err - } - - // detach the previous role policy - // If noSuchEntity error, we can skip - if _, err := awsClient.DetachRolePolicy(&iam.DetachRolePolicyInput{ - PolicyArn: &policyArn, - RoleName: &roleName, - }); err != nil { - if aerr, ok := err.(awserr.Error); ok { - if aerr.Code() != iam.ErrCodeNoSuchEntityException { - return err - } - } else { - return err - } - } - - // delete the previous policy - // If noSuchEntity error, we can skip - if _, err := awsClient.DeletePolicy(&iam.DeletePolicyInput{ - PolicyArn: &policyArn, - }); err != nil { - if aerr, ok := err.(awserr.Error); ok { - if aerr.Code() != iam.ErrCodeNoSuchEntityException { - return err - } - } else { - return err - } - } - - // create new policy - backoff := wait.Backoff{Duration: time.Second, Factor: 2, Steps: 3} - if err := wait.ExponentialBackoff(backoff, func() (done bool, err error) { - if _, err := awsClient.CreatePolicy(&iam.CreatePolicyInput{ - PolicyName: &policyName, - Description: &federatedRole.Spec.AWSCustomPolicy.Description, - PolicyDocument: aws.String(string(payload)), - }); err != nil { - if aerr, ok := err.(awserr.Error); ok { - if aerr.Code() == iam.ErrCodeEntityAlreadyExistsException { - return false, nil - } - } else { - return false, err - } - } - return true, nil - }); err != nil { - return err - } - - rolePolicies, err := awsClient.ListAttachedRolePolicies(&iam.ListAttachedRolePoliciesInput{ - RoleName: &roleName, - }) - if err != nil { - return err - } - - policiesMap := make(map[string]struct{}) - for _, policy := range federatedRole.Spec.AWSManagedPolicies { - policiesMap[policy] = struct{}{} - } - - // two slices for polices/policyArns that need to attach and detach - policyArnsToAttach := []string{policyArn} - policyArnsToDetach := make([]string, 0) - - // check existing role policies - for _, policy := range rolePolicies.AttachedPolicies { - if _, ok := policiesMap[*policy.PolicyName]; ok { - delete(policiesMap, *policy.PolicyName) - } else { - policyArnsToDetach = append(policyArnsToDetach, *policy.PolicyArn) - } - } - - for policy := range policiesMap { - policyArnsToAttach = append(policyArnsToAttach, "arn:aws:iam::aws:policy/"+policy) - } - - // Detach all removed polices from role - for _, policyArn := range policyArnsToDetach { - policyArn := policyArn // fix for implicit memory aliasing - if _, err := awsClient.DetachRolePolicy(&iam.DetachRolePolicyInput{ - PolicyArn: &policyArn, - RoleName: &roleName, - }); err != nil { - if aerr, ok := err.(awserr.Error); ok { - if aerr.Code() != iam.ErrCodeNoSuchEntityException { - return err - } - } else { - return err - } - } - } - - // Attach all needed polices to role - for _, policyArn := range policyArnsToAttach { - policyArn := policyArn // fix for implicit memory aliasing - if _, err := awsClient.AttachRolePolicy(&iam.AttachRolePolicyInput{ - RoleName: &roleName, - PolicyArn: &policyArn, - }); err != nil { - return err - } - } - - return nil -} - func GenerateRoleARN(accountId, roleName string) string { return fmt.Sprintf("arn:aws:iam::%s:role/%s", accountId, roleName) } diff --git a/pkg/provider/aws/iam_test.go b/pkg/provider/aws/iam_test.go index 2f8b6205..c8b85f90 100644 --- a/pkg/provider/aws/iam_test.go +++ b/pkg/provider/aws/iam_test.go @@ -10,9 +10,6 @@ import ( "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/iam" "github.com/golang/mock/gomock" - awsv1alpha1 "github.com/openshift/aws-account-operator/api/v1alpha1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/openshift/osdctl/pkg/provider/aws/mock" ) @@ -254,307 +251,3 @@ func TestCreateIAMUserAndAttachPolicy(t *testing.T) { }) } } - -func TestRefreshIAMPolicy(t *testing.T) { - g := NewGomegaWithT(t) - - exampleFederatedRole := &awsv1alpha1.AWSFederatedRole{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - }, - Spec: awsv1alpha1.AWSFederatedRoleSpec{ - AWSCustomPolicy: awsv1alpha1.AWSCustomPolicy{ - Name: "bar", - Description: "foo", - Statements: []awsv1alpha1.StatementEntry{ - { - Effect: "Allow", - Action: []string{"aws-portal:ViewAccount"}, - Resource: []string{"*"}, - }, - }, - }, - }, - } - - exampleFederatedRoleWithManagedPolices := &awsv1alpha1.AWSFederatedRole{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - }, - Spec: awsv1alpha1.AWSFederatedRoleSpec{ - AWSCustomPolicy: awsv1alpha1.AWSCustomPolicy{ - Name: "bar", - Description: "foo", - Statements: []awsv1alpha1.StatementEntry{ - { - Effect: "Allow", - Action: []string{"aws-portal:ViewAccount"}, - Resource: []string{"*"}, - }, - }, - }, - AWSManagedPolicies: []string{ - "foo", - "bar", - }, - }, - } - - testCases := []struct { - title string - setupAWSMock func(r *mock.MockClientMockRecorder) - federatedRole *awsv1alpha1.AWSFederatedRole - accountID string - uid string - errExpected bool - }{ - { - title: "Failed to detach role policy", - setupAWSMock: func(r *mock.MockClientMockRecorder) { - r.DetachRolePolicy(gomock.Any()).Return(nil, errors.New("FakeError")).Times(1) - }, - federatedRole: exampleFederatedRole, - accountID: "foo", - uid: "bar", - errExpected: true, - }, - { - title: "Failed to delete policy", - setupAWSMock: func(r *mock.MockClientMockRecorder) { - gomock.InOrder( - r.DetachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.DeletePolicy(gomock.Any()).Return(nil, errors.New("FakeError")).Times(1), - ) - }, - federatedRole: exampleFederatedRole, - accountID: "foo", - uid: "bar", - errExpected: true, - }, - { - title: "Failed to create policy", - setupAWSMock: func(r *mock.MockClientMockRecorder) { - gomock.InOrder( - r.DetachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.DeletePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.CreatePolicy(gomock.Any()).Return(nil, errors.New("FakeError")).Times(1), - ) - }, - federatedRole: exampleFederatedRole, - accountID: "foo", - uid: "bar", - errExpected: true, - }, - { - title: "Failed to list current role policies", - setupAWSMock: func(r *mock.MockClientMockRecorder) { - gomock.InOrder( - r.DetachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.DeletePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.CreatePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.ListAttachedRolePolicies(gomock.Any()).Return(nil, errors.New("FakeError")).Times(1), - ) - }, - federatedRole: exampleFederatedRole, - accountID: "foo", - uid: "bar", - errExpected: true, - }, - { - title: "Failed to attach role policy", - setupAWSMock: func(r *mock.MockClientMockRecorder) { - gomock.InOrder( - r.DetachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.DeletePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.CreatePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.ListAttachedRolePolicies(gomock.Any()).Return( - &iam.ListAttachedRolePoliciesOutput{ - AttachedPolicies: []*iam.AttachedPolicy{}}, nil).Times(1), - r.AttachRolePolicy(gomock.Any()).Return(nil, errors.New("FakeError")).Times(1), - ) - }, - federatedRole: exampleFederatedRole, - accountID: "foo", - uid: "bar", - errExpected: true, - }, - { - title: "Detach role policy noSuchEntity error", - setupAWSMock: func(r *mock.MockClientMockRecorder) { - gomock.InOrder( - r.DetachRolePolicy(gomock.Any()). - Return(nil, awserr.New(iam.ErrCodeNoSuchEntityException, "", - errors.New("FakeError"))).Times(1), - r.DeletePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.CreatePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.ListAttachedRolePolicies(gomock.Any()).Return( - &iam.ListAttachedRolePoliciesOutput{ - AttachedPolicies: []*iam.AttachedPolicy{}}, nil).Times(1), - r.AttachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - ) - }, - federatedRole: exampleFederatedRole, - accountID: "foo", - uid: "bar", - errExpected: false, - }, - { - title: "Detach role policy noSuchEntity error", - setupAWSMock: func(r *mock.MockClientMockRecorder) { - gomock.InOrder( - r.DetachRolePolicy(gomock.Any()). - Return(nil, awserr.New(iam.ErrCodeNoSuchEntityException, "", - errors.New("FakeError"))).Times(1), - r.DeletePolicy(gomock.Any()). - Return(nil, awserr.New(iam.ErrCodeNoSuchEntityException, "", - errors.New("FakeError"))).Times(1), - r.CreatePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.ListAttachedRolePolicies(gomock.Any()).Return( - &iam.ListAttachedRolePoliciesOutput{ - AttachedPolicies: []*iam.AttachedPolicy{}}, nil).Times(1), - r.AttachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - ) - }, - federatedRole: exampleFederatedRole, - accountID: "foo", - uid: "bar", - errExpected: false, - }, - { - title: "Retry entity already exists error", - setupAWSMock: func(r *mock.MockClientMockRecorder) { - gomock.InOrder( - r.DetachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.DeletePolicy(gomock.Any()).Return(nil, nil).Times(1), - // Retry one time - r.CreatePolicy(gomock.Any()).Return(nil, - awserr.New(iam.ErrCodeEntityAlreadyExistsException, "", - errors.New("FakeError"))).Times(1), - r.CreatePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.ListAttachedRolePolicies(gomock.Any()).Return( - &iam.ListAttachedRolePoliciesOutput{ - AttachedPolicies: []*iam.AttachedPolicy{}}, nil).Times(1), - r.AttachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - ) - }, - federatedRole: exampleFederatedRole, - accountID: "foo", - uid: "bar", - errExpected: false, - }, - { - title: "success with none policy names", - setupAWSMock: func(r *mock.MockClientMockRecorder) { - gomock.InOrder( - r.DetachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.DeletePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.CreatePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.ListAttachedRolePolicies(gomock.Any()).Return( - &iam.ListAttachedRolePoliciesOutput{ - AttachedPolicies: []*iam.AttachedPolicy{}}, nil).Times(1), - r.AttachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - ) - }, - federatedRole: exampleFederatedRole, - accountID: "foo", - uid: "bar", - errExpected: false, - }, - { - title: "need to remove outdated policies", - setupAWSMock: func(r *mock.MockClientMockRecorder) { - gomock.InOrder( - r.DetachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.DeletePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.CreatePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.ListAttachedRolePolicies(gomock.Any()).Return( - &iam.ListAttachedRolePoliciesOutput{ - AttachedPolicies: []*iam.AttachedPolicy{ - { - PolicyArn: aws.String("foo"), - PolicyName: aws.String("foo"), - }, - { - PolicyArn: aws.String("bar"), - PolicyName: aws.String("bar"), - }, - }}, nil).Times(1), - r.DetachRolePolicy(gomock.Any()).Return(nil, nil).Times(2), - r.AttachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - ) - }, - federatedRole: exampleFederatedRole, - accountID: "foo", - uid: "bar", - errExpected: false, - }, - { - title: "don't need to remove managed policies", - setupAWSMock: func(r *mock.MockClientMockRecorder) { - gomock.InOrder( - r.DetachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.DeletePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.CreatePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.ListAttachedRolePolicies(gomock.Any()).Return( - &iam.ListAttachedRolePoliciesOutput{ - AttachedPolicies: []*iam.AttachedPolicy{ - { - PolicyName: aws.String("foo"), - PolicyArn: aws.String("foo"), - }, - { - PolicyName: aws.String("bar"), - PolicyArn: aws.String("bar"), - }, - }}, nil).Times(1), - r.AttachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - ) - }, - federatedRole: exampleFederatedRoleWithManagedPolices, - accountID: "foo", - uid: "bar", - errExpected: false, - }, - { - title: "need to add new managed policies", - setupAWSMock: func(r *mock.MockClientMockRecorder) { - gomock.InOrder( - r.DetachRolePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.DeletePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.CreatePolicy(gomock.Any()).Return(nil, nil).Times(1), - r.ListAttachedRolePolicies(gomock.Any()).Return( - &iam.ListAttachedRolePoliciesOutput{ - AttachedPolicies: []*iam.AttachedPolicy{ - { - PolicyName: aws.String("foo"), - PolicyArn: aws.String("bar"), - }, - }}, nil).Times(1), - r.AttachRolePolicy(gomock.Any()).Return(nil, nil).Times(2), - ) - }, - federatedRole: exampleFederatedRoleWithManagedPolices, - accountID: "foo", - uid: "bar", - errExpected: false, - }, - } - - for _, tc := range testCases { - t.Run(tc.title, func(t *testing.T) { - mocks := setupDefaultMocks(t) - tc.setupAWSMock(mocks.mockAWSClient.EXPECT()) - - // This is necessary for the mocks to report failures like methods not being called an expected number of times. - // after mocks is defined - defer mocks.mockCtrl.Finish() - - err := RefreshIAMPolicy(mocks.mockAWSClient, tc.federatedRole, tc.accountID, tc.uid) - if tc.errExpected { - g.Expect(err).Should(HaveOccurred()) - } else { - g.Expect(err).ShouldNot(HaveOccurred()) - } - }) - } -}