From 7ba9b0b980de58743b0803769406ae5f6fe249c0 Mon Sep 17 00:00:00 2001 From: Janice Date: Mon, 26 Sep 2022 15:17:56 -0700 Subject: [PATCH 1/7] chore: add perm bound validation --- internal/pkg/aws/iam/iam.go | 15 ++++++ internal/pkg/aws/iam/iam_test.go | 61 +++++++++++++++++++++++ internal/pkg/aws/iam/mocks/mock_iam.go | 15 ++++++ internal/pkg/cli/app_init.go | 21 ++++++++ internal/pkg/cli/app_init_test.go | 31 ++++++++++-- internal/pkg/cli/interfaces.go | 4 ++ internal/pkg/cli/mocks/mock_interfaces.go | 38 ++++++++++++++ 7 files changed, 181 insertions(+), 4 deletions(-) diff --git a/internal/pkg/aws/iam/iam.go b/internal/pkg/aws/iam/iam.go index 92e674449d2..765895dd8cd 100644 --- a/internal/pkg/aws/iam/iam.go +++ b/internal/pkg/aws/iam/iam.go @@ -25,6 +25,7 @@ type api interface { ListRolePolicies(input *iam.ListRolePoliciesInput) (*iam.ListRolePoliciesOutput, error) DeleteRole(input *iam.DeleteRoleInput) (*iam.DeleteRoleOutput, error) CreateServiceLinkedRole(input *iam.CreateServiceLinkedRoleInput) (*iam.CreateServiceLinkedRoleOutput, error) + ListPolicies(input *iam.ListPoliciesInput) (*iam.ListPoliciesOutput, error) } // IAM wraps the AWS SDK's IAM client. @@ -98,6 +99,20 @@ func (c *IAM) CreateECSServiceLinkedRole() error { return nil } +func (c *IAM) ListPolicies() ([]*string, error) { + policies, err := c.client.ListPolicies(&iam.ListPoliciesInput{ + PolicyUsageFilter: aws.String("PermissionsBoundary"), + }) + if err != nil { + return nil, fmt.Errorf("list IAM policies: %w", err) + } + var policyNames = make([]*string, len(policies.Policies)) + for i, policy := range policies.Policies { + policyNames[i] = policy.PolicyName + } + return policyNames, nil +} + func (c *IAM) deleteRolePolicies(roleName string) error { policyNames, err := c.listRolePolicyNames(roleName) if err != nil { diff --git a/internal/pkg/aws/iam/iam_test.go b/internal/pkg/aws/iam/iam_test.go index 7223baf0d2f..469693d9700 100644 --- a/internal/pkg/aws/iam/iam_test.go +++ b/internal/pkg/aws/iam/iam_test.go @@ -242,3 +242,64 @@ func TestIAM_CreateECSServiceLinkedRole(t *testing.T) { }) } } + +func TestIAM_ListPolicies(t *testing.T) { + testCases := map[string]struct { + inClient func(ctrl *gomock.Controller) *mocks.Mockapi + + wantedPolicies []*string + wantedErr error + }{ + "wraps error on failure": { + inClient: func(ctrl *gomock.Controller) *mocks.Mockapi { + m := mocks.NewMockapi(ctrl) + m.EXPECT(). + ListPolicies(gomock.Any()). + Return(nil, errors.New("some error")) + return m + }, + + wantedErr: errors.New("list IAM policies: some error"), + }, + "returns list of policy names": { + inClient: func(ctrl *gomock.Controller) *mocks.Mockapi { + m := mocks.NewMockapi(ctrl) + m.EXPECT(). + ListPolicies(gomock.Any()). + Return(&iam.ListPoliciesOutput{ + Policies: []*iam.Policy{ + { + PolicyName: aws.String("myFirstPolicyName"), + }, + { + PolicyName: aws.String("mySecondPolicyName"), + }, + }, + }, nil) + return m + }, + wantedPolicies: []*string{aws.String("myFirstPolicyName"), aws.String("mySecondPolicyName")}, + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // GIVEN + ctrl := gomock.NewController(t) + defer ctrl.Finish() + iam := &IAM{ + client: tc.inClient(ctrl), + } + + // WHEN + output, err := iam.ListPolicies() + + // THEN + if tc.wantedErr != nil { + require.EqualError(t, err, tc.wantedErr.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantedPolicies, output) + } + }) + } +} diff --git a/internal/pkg/aws/iam/mocks/mock_iam.go b/internal/pkg/aws/iam/mocks/mock_iam.go index 0858de5b034..973b3781dd3 100644 --- a/internal/pkg/aws/iam/mocks/mock_iam.go +++ b/internal/pkg/aws/iam/mocks/mock_iam.go @@ -79,6 +79,21 @@ func (mr *MockapiMockRecorder) DeleteRolePolicy(input interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteRolePolicy", reflect.TypeOf((*Mockapi)(nil).DeleteRolePolicy), input) } +// ListPolicies mocks base method. +func (m *Mockapi) ListPolicies(input *iam.ListPoliciesInput) (*iam.ListPoliciesOutput, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListPolicies", input) + ret0, _ := ret[0].(*iam.ListPoliciesOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListPolicies indicates an expected call of ListPolicies. +func (mr *MockapiMockRecorder) ListPolicies(input interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListPolicies", reflect.TypeOf((*Mockapi)(nil).ListPolicies), input) +} + // ListRolePolicies mocks base method. func (m *Mockapi) ListRolePolicies(input *iam.ListRolePoliciesInput) (*iam.ListRolePoliciesOutput, error) { m.ctrl.T.Helper() diff --git a/internal/pkg/cli/app_init.go b/internal/pkg/cli/app_init.go index c97a45514a3..21377d37448 100644 --- a/internal/pkg/cli/app_init.go +++ b/internal/pkg/cli/app_init.go @@ -6,6 +6,7 @@ package cli import ( "errors" "fmt" + "github.com/aws/copilot-cli/internal/pkg/aws/iam" "os" "github.com/aws/aws-sdk-go/aws" @@ -51,6 +52,7 @@ type initAppOpts struct { cfn appDeployer prompt prompter prog progress + iam policyLister isSessionFromEnvVars func() (bool, error) cachedHostedZoneID string @@ -77,6 +79,7 @@ func newInitAppOpts(vars initAppVars) (*initAppOpts, error) { cfn: cloudformation.New(sess, cloudformation.WithProgressTracker(os.Stderr)), prompt: prompt.New(), prog: termprogress.NewSpinner(log.DiagnosticWriter), + iam: iam.New(sess), isSessionFromEnvVars: func() (bool, error) { return sessions.AreCredsFromEnvVars(sess) }, @@ -90,6 +93,11 @@ func (o *initAppOpts) Validate() error { return err } } + if o.permissionsBoundary != "" { + if err := o.validatePermBound(o.permissionsBoundary); err != nil { + return err + } + } if o.domainName != "" { if err := validateDomainName(o.domainName); err != nil { return fmt.Errorf("domain name %s is invalid: %w", o.domainName, err) @@ -234,6 +242,19 @@ func (o *initAppOpts) validateAppName(name string) error { return nil } +func (o *initAppOpts) validatePermBound(policyName string) error { + IAMPolicies, err := o.iam.ListPolicies() + if err != nil { + return fmt.Errorf("list permissions boundary policies: %w", err) + } + for _, policy := range IAMPolicies { + if aws.StringValue(policy) == policyName { + return nil + } + } + return fmt.Errorf("IAM policy '%s' not found in this account", policyName) +} + func (o *initAppOpts) isDomainOwned() error { err := o.domainInfoGetter.IsRegisteredDomain(o.domainName) if err == nil { diff --git a/internal/pkg/cli/app_init_test.go b/internal/pkg/cli/app_init_test.go index 5ac5ea9c64d..1c21abf0554 100644 --- a/internal/pkg/cli/app_init_test.go +++ b/internal/pkg/cli/app_init_test.go @@ -6,6 +6,7 @@ package cli import ( "errors" "fmt" + "github.com/aws/aws-sdk-go/aws" "testing" "github.com/aws/copilot-cli/internal/pkg/aws/identity" @@ -22,12 +23,14 @@ type initAppMocks struct { mockRoute53Svc *mocks.MockdomainHostedZoneGetter mockStore *mocks.Mockstore mockDomainInfoGetter *mocks.MockdomainInfoGetter + mockPolicyLister *mocks.MockpolicyLister } func TestInitAppOpts_Validate(t *testing.T) { testCases := map[string]struct { - inAppName string - inDomainName string + inAppName string + inDomainName string + inPBPolicyName string mock func(m *initAppMocks) @@ -90,6 +93,23 @@ func TestInitAppOpts_Validate(t *testing.T) { }, wantedError: errors.New("check if domain is owned by the account: some error"), }, + "wrap error from ListPolicies": { + inPBPolicyName: "nonexistentPolicyName", + mock: func(m *initAppMocks) { + m.mockPolicyLister.EXPECT().ListPolicies().Return(nil, errors.New("some error")) + }, + wantedError: errors.New("list permissions boundary policies: some error"), + }, + "invalid permissions boundary policy name": { + inPBPolicyName: "nonexistentPolicyName", + mock: func(m *initAppMocks) { + m.mockPolicyLister.EXPECT().ListPolicies().Return( + []*string{ + aws.String("existentPolicyName"), + }, nil) + }, + wantedError: errors.New("IAM policy 'nonexistentPolicyName' not found in this account"), + }, "invalid domain name that doesn't have a hosted zone": { inDomainName: "badMockDomain.com", mock: func(m *initAppMocks) { @@ -132,6 +152,7 @@ func TestInitAppOpts_Validate(t *testing.T) { mockStore: mocks.NewMockstore(ctrl), mockRoute53Svc: mocks.NewMockdomainHostedZoneGetter(ctrl), mockDomainInfoGetter: mocks.NewMockdomainInfoGetter(ctrl), + mockPolicyLister: mocks.NewMockpolicyLister(ctrl), } tc.mock(m) @@ -139,9 +160,11 @@ func TestInitAppOpts_Validate(t *testing.T) { route53: m.mockRoute53Svc, domainInfoGetter: m.mockDomainInfoGetter, store: m.mockStore, + iam: m.mockPolicyLister, initAppVars: initAppVars{ - name: tc.inAppName, - domainName: tc.inDomainName, + name: tc.inAppName, + domainName: tc.inDomainName, + permissionsBoundary: tc.inPBPolicyName, }, } diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index 59d294106df..600c65f926c 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -555,6 +555,10 @@ type roleDeleter interface { DeleteRole(string) error } +type policyLister interface { + ListPolicies() ([]*string, error) +} + type serviceDescriber interface { DescribeService(app, env, svc string) (*ecs.ServiceDesc, error) } diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index 7d30f3aa3a7..8749fc72e0f 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -5891,6 +5891,44 @@ func (mr *MockroleDeleterMockRecorder) DeleteRole(arg0 interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteRole", reflect.TypeOf((*MockroleDeleter)(nil).DeleteRole), arg0) } +// MockpolicyLister is a mock of policyLister interface. +type MockpolicyLister struct { + ctrl *gomock.Controller + recorder *MockpolicyListerMockRecorder +} + +// MockpolicyListerMockRecorder is the mock recorder for MockpolicyLister. +type MockpolicyListerMockRecorder struct { + mock *MockpolicyLister +} + +// NewMockpolicyLister creates a new mock instance. +func NewMockpolicyLister(ctrl *gomock.Controller) *MockpolicyLister { + mock := &MockpolicyLister{ctrl: ctrl} + mock.recorder = &MockpolicyListerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockpolicyLister) EXPECT() *MockpolicyListerMockRecorder { + return m.recorder +} + +// ListPolicies mocks base method. +func (m *MockpolicyLister) ListPolicies() ([]*string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListPolicies") + ret0, _ := ret[0].([]*string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListPolicies indicates an expected call of ListPolicies. +func (mr *MockpolicyListerMockRecorder) ListPolicies() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListPolicies", reflect.TypeOf((*MockpolicyLister)(nil).ListPolicies)) +} + // MockserviceDescriber is a mock of serviceDescriber interface. type MockserviceDescriber struct { ctrl *gomock.Controller From 5a76348938d938450774e86560b25b27480134b1 Mon Sep 17 00:00:00 2001 From: Janice Date: Mon, 26 Sep 2022 15:39:48 -0700 Subject: [PATCH 2/7] chore: rename method --- internal/pkg/aws/iam/iam.go | 3 ++- internal/pkg/aws/iam/iam_test.go | 2 +- internal/pkg/cli/app_init.go | 2 +- internal/pkg/cli/app_init_test.go | 4 ++-- internal/pkg/cli/interfaces.go | 2 +- internal/pkg/cli/mocks/mock_interfaces.go | 12 ++++++------ 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/internal/pkg/aws/iam/iam.go b/internal/pkg/aws/iam/iam.go index 765895dd8cd..a13382bb77f 100644 --- a/internal/pkg/aws/iam/iam.go +++ b/internal/pkg/aws/iam/iam.go @@ -99,7 +99,8 @@ func (c *IAM) CreateECSServiceLinkedRole() error { return nil } -func (c *IAM) ListPolicies() ([]*string, error) { +// ListPermBoundPolicyNames returns a list of permissions boundary policy names. +func (c *IAM) ListPermBoundPolicyNames() ([]*string, error) { policies, err := c.client.ListPolicies(&iam.ListPoliciesInput{ PolicyUsageFilter: aws.String("PermissionsBoundary"), }) diff --git a/internal/pkg/aws/iam/iam_test.go b/internal/pkg/aws/iam/iam_test.go index 469693d9700..e87092ed84a 100644 --- a/internal/pkg/aws/iam/iam_test.go +++ b/internal/pkg/aws/iam/iam_test.go @@ -291,7 +291,7 @@ func TestIAM_ListPolicies(t *testing.T) { } // WHEN - output, err := iam.ListPolicies() + output, err := iam.ListPermBoundPolicyNames() // THEN if tc.wantedErr != nil { diff --git a/internal/pkg/cli/app_init.go b/internal/pkg/cli/app_init.go index 21377d37448..18991a0df0e 100644 --- a/internal/pkg/cli/app_init.go +++ b/internal/pkg/cli/app_init.go @@ -243,7 +243,7 @@ func (o *initAppOpts) validateAppName(name string) error { } func (o *initAppOpts) validatePermBound(policyName string) error { - IAMPolicies, err := o.iam.ListPolicies() + IAMPolicies, err := o.iam.ListPermBoundPolicyNames() if err != nil { return fmt.Errorf("list permissions boundary policies: %w", err) } diff --git a/internal/pkg/cli/app_init_test.go b/internal/pkg/cli/app_init_test.go index 1c21abf0554..160e4afb09e 100644 --- a/internal/pkg/cli/app_init_test.go +++ b/internal/pkg/cli/app_init_test.go @@ -96,14 +96,14 @@ func TestInitAppOpts_Validate(t *testing.T) { "wrap error from ListPolicies": { inPBPolicyName: "nonexistentPolicyName", mock: func(m *initAppMocks) { - m.mockPolicyLister.EXPECT().ListPolicies().Return(nil, errors.New("some error")) + m.mockPolicyLister.EXPECT().ListPermBoundPolicyNames().Return(nil, errors.New("some error")) }, wantedError: errors.New("list permissions boundary policies: some error"), }, "invalid permissions boundary policy name": { inPBPolicyName: "nonexistentPolicyName", mock: func(m *initAppMocks) { - m.mockPolicyLister.EXPECT().ListPolicies().Return( + m.mockPolicyLister.EXPECT().ListPermBoundPolicyNames().Return( []*string{ aws.String("existentPolicyName"), }, nil) diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index 600c65f926c..4a7cb4f2f35 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -556,7 +556,7 @@ type roleDeleter interface { } type policyLister interface { - ListPolicies() ([]*string, error) + ListPermBoundPolicyNames() ([]*string, error) } type serviceDescriber interface { diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index 8749fc72e0f..98df2b5617b 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -5914,19 +5914,19 @@ func (m *MockpolicyLister) EXPECT() *MockpolicyListerMockRecorder { return m.recorder } -// ListPolicies mocks base method. -func (m *MockpolicyLister) ListPolicies() ([]*string, error) { +// ListPermBoundPolicyNames mocks base method. +func (m *MockpolicyLister) ListPermBoundPolicyNames() ([]*string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ListPolicies") + ret := m.ctrl.Call(m, "ListPermBoundPolicyNames") ret0, _ := ret[0].([]*string) ret1, _ := ret[1].(error) return ret0, ret1 } -// ListPolicies indicates an expected call of ListPolicies. -func (mr *MockpolicyListerMockRecorder) ListPolicies() *gomock.Call { +// ListPermBoundPolicyNames indicates an expected call of ListPermBoundPolicyNames. +func (mr *MockpolicyListerMockRecorder) ListPermBoundPolicyNames() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListPolicies", reflect.TypeOf((*MockpolicyLister)(nil).ListPolicies)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListPermBoundPolicyNames", reflect.TypeOf((*MockpolicyLister)(nil).ListPermBoundPolicyNames)) } // MockserviceDescriber is a mock of serviceDescriber interface. From 9463801f769aff602a446a76d2b86a5d85c58216 Mon Sep 17 00:00:00 2001 From: Janice Date: Tue, 27 Sep 2022 14:09:42 -0700 Subject: [PATCH 3/7] chore: unpoint string --- internal/pkg/aws/iam/iam.go | 8 +++++--- internal/pkg/aws/iam/iam_test.go | 4 ++-- internal/pkg/cli/app_init.go | 2 +- internal/pkg/cli/interfaces.go | 2 +- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/pkg/aws/iam/iam.go b/internal/pkg/aws/iam/iam.go index a13382bb77f..033154e0683 100644 --- a/internal/pkg/aws/iam/iam.go +++ b/internal/pkg/aws/iam/iam.go @@ -100,16 +100,18 @@ func (c *IAM) CreateECSServiceLinkedRole() error { } // ListPermBoundPolicyNames returns a list of permissions boundary policy names. -func (c *IAM) ListPermBoundPolicyNames() ([]*string, error) { +func (c *IAM) ListPermBoundPolicyNames() ([]string, error) { policies, err := c.client.ListPolicies(&iam.ListPoliciesInput{ PolicyUsageFilter: aws.String("PermissionsBoundary"), + Scope: aws.String("Local"), }) if err != nil { return nil, fmt.Errorf("list IAM policies: %w", err) } - var policyNames = make([]*string, len(policies.Policies)) + var policyNames = make([]string, len(policies.Policies)) for i, policy := range policies.Policies { - policyNames[i] = policy.PolicyName + fmt.Println(aws.StringValue(policy.PolicyName)) + policyNames[i] = aws.StringValue(policy.PolicyName) } return policyNames, nil } diff --git a/internal/pkg/aws/iam/iam_test.go b/internal/pkg/aws/iam/iam_test.go index e87092ed84a..5386984af79 100644 --- a/internal/pkg/aws/iam/iam_test.go +++ b/internal/pkg/aws/iam/iam_test.go @@ -247,7 +247,7 @@ func TestIAM_ListPolicies(t *testing.T) { testCases := map[string]struct { inClient func(ctrl *gomock.Controller) *mocks.Mockapi - wantedPolicies []*string + wantedPolicies []string wantedErr error }{ "wraps error on failure": { @@ -278,7 +278,7 @@ func TestIAM_ListPolicies(t *testing.T) { }, nil) return m }, - wantedPolicies: []*string{aws.String("myFirstPolicyName"), aws.String("mySecondPolicyName")}, + wantedPolicies: []string{"myFirstPolicyName", "mySecondPolicyName"}, }, } for name, tc := range testCases { diff --git a/internal/pkg/cli/app_init.go b/internal/pkg/cli/app_init.go index 18991a0df0e..6bec21eb72b 100644 --- a/internal/pkg/cli/app_init.go +++ b/internal/pkg/cli/app_init.go @@ -248,7 +248,7 @@ func (o *initAppOpts) validatePermBound(policyName string) error { return fmt.Errorf("list permissions boundary policies: %w", err) } for _, policy := range IAMPolicies { - if aws.StringValue(policy) == policyName { + if policy == policyName { return nil } } diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index 4a7cb4f2f35..b34cebf9f57 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -556,7 +556,7 @@ type roleDeleter interface { } type policyLister interface { - ListPermBoundPolicyNames() ([]*string, error) + ListPermBoundPolicyNames() ([]string, error) } type serviceDescriber interface { From 07110ff762fea1a92f31fbc17b9c8f31e30420aa Mon Sep 17 00:00:00 2001 From: Janice Date: Tue, 27 Sep 2022 14:52:45 -0700 Subject: [PATCH 4/7] chore: user marker to paginate response --- internal/pkg/aws/iam/iam.go | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/internal/pkg/aws/iam/iam.go b/internal/pkg/aws/iam/iam.go index 033154e0683..705ec875cdb 100644 --- a/internal/pkg/aws/iam/iam.go +++ b/internal/pkg/aws/iam/iam.go @@ -101,16 +101,25 @@ func (c *IAM) CreateECSServiceLinkedRole() error { // ListPermBoundPolicyNames returns a list of permissions boundary policy names. func (c *IAM) ListPermBoundPolicyNames() ([]string, error) { - policies, err := c.client.ListPolicies(&iam.ListPoliciesInput{ - PolicyUsageFilter: aws.String("PermissionsBoundary"), - Scope: aws.String("Local"), - }) - if err != nil { - return nil, fmt.Errorf("list IAM policies: %w", err) + var policies []*iam.Policy + var marker *string + for { + output, err := c.client.ListPolicies(&iam.ListPoliciesInput{ + Marker: marker, + Scope: aws.String("Local"), + PolicyUsageFilter: aws.String("PermissionsBoundary"), + }) + if err != nil { + return nil, fmt.Errorf("list IAM policies: %w", err) + } + policies = append(policies, output.Policies...) + if !aws.BoolValue(output.IsTruncated) { + break + } + marker = output.Marker } - var policyNames = make([]string, len(policies.Policies)) - for i, policy := range policies.Policies { - fmt.Println(aws.StringValue(policy.PolicyName)) + var policyNames = make([]string, len(policies)) + for i, policy := range policies { policyNames[i] = aws.StringValue(policy.PolicyName) } return policyNames, nil From 449cb5cbd6d3341c9e3c0ef030e35eff28e103d5 Mon Sep 17 00:00:00 2001 From: Janice Date: Tue, 27 Sep 2022 14:59:04 -0700 Subject: [PATCH 5/7] chore: update mocks --- internal/pkg/cli/mocks/mock_interfaces.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index 98df2b5617b..017b167fe2b 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -5915,10 +5915,10 @@ func (m *MockpolicyLister) EXPECT() *MockpolicyListerMockRecorder { } // ListPermBoundPolicyNames mocks base method. -func (m *MockpolicyLister) ListPermBoundPolicyNames() ([]*string, error) { +func (m *MockpolicyLister) ListPermBoundPolicyNames() ([]string, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ListPermBoundPolicyNames") - ret0, _ := ret[0].([]*string) + ret0, _ := ret[0].([]string) ret1, _ := ret[1].(error) return ret0, ret1 } From cd3a93eca4dcd45c5a21f972797d8e23a5ac6750 Mon Sep 17 00:00:00 2001 From: Janice Date: Tue, 27 Sep 2022 15:09:27 -0700 Subject: [PATCH 6/7] chore: update unit test --- internal/pkg/cli/app_init_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/internal/pkg/cli/app_init_test.go b/internal/pkg/cli/app_init_test.go index 160e4afb09e..87c35b123e4 100644 --- a/internal/pkg/cli/app_init_test.go +++ b/internal/pkg/cli/app_init_test.go @@ -6,7 +6,6 @@ package cli import ( "errors" "fmt" - "github.com/aws/aws-sdk-go/aws" "testing" "github.com/aws/copilot-cli/internal/pkg/aws/identity" @@ -104,9 +103,7 @@ func TestInitAppOpts_Validate(t *testing.T) { inPBPolicyName: "nonexistentPolicyName", mock: func(m *initAppMocks) { m.mockPolicyLister.EXPECT().ListPermBoundPolicyNames().Return( - []*string{ - aws.String("existentPolicyName"), - }, nil) + []string{"existentPolicyName"}, nil) }, wantedError: errors.New("IAM policy 'nonexistentPolicyName' not found in this account"), }, From 166abb059797b10d91125f5aac2226c724dff20e Mon Sep 17 00:00:00 2001 From: Janice Date: Tue, 4 Oct 2022 15:14:44 -0700 Subject: [PATCH 7/7] chore: address fb --- internal/pkg/aws/iam/iam.go | 4 ++-- internal/pkg/aws/iam/iam_test.go | 2 +- internal/pkg/cli/app_init.go | 4 ++-- internal/pkg/cli/app_init_test.go | 6 +++--- internal/pkg/cli/interfaces.go | 2 +- internal/pkg/cli/mocks/mock_interfaces.go | 12 ++++++------ 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/pkg/aws/iam/iam.go b/internal/pkg/aws/iam/iam.go index 705ec875cdb..44232140bbe 100644 --- a/internal/pkg/aws/iam/iam.go +++ b/internal/pkg/aws/iam/iam.go @@ -99,8 +99,8 @@ func (c *IAM) CreateECSServiceLinkedRole() error { return nil } -// ListPermBoundPolicyNames returns a list of permissions boundary policy names. -func (c *IAM) ListPermBoundPolicyNames() ([]string, error) { +// ListPolicyNames returns a list of local policy names. +func (c *IAM) ListPolicyNames() ([]string, error) { var policies []*iam.Policy var marker *string for { diff --git a/internal/pkg/aws/iam/iam_test.go b/internal/pkg/aws/iam/iam_test.go index 5386984af79..705f8da0bb6 100644 --- a/internal/pkg/aws/iam/iam_test.go +++ b/internal/pkg/aws/iam/iam_test.go @@ -291,7 +291,7 @@ func TestIAM_ListPolicies(t *testing.T) { } // WHEN - output, err := iam.ListPermBoundPolicyNames() + output, err := iam.ListPolicyNames() // THEN if tc.wantedErr != nil { diff --git a/internal/pkg/cli/app_init.go b/internal/pkg/cli/app_init.go index 6bec21eb72b..3126348d175 100644 --- a/internal/pkg/cli/app_init.go +++ b/internal/pkg/cli/app_init.go @@ -243,7 +243,7 @@ func (o *initAppOpts) validateAppName(name string) error { } func (o *initAppOpts) validatePermBound(policyName string) error { - IAMPolicies, err := o.iam.ListPermBoundPolicyNames() + IAMPolicies, err := o.iam.ListPolicyNames() if err != nil { return fmt.Errorf("list permissions boundary policies: %w", err) } @@ -252,7 +252,7 @@ func (o *initAppOpts) validatePermBound(policyName string) error { return nil } } - return fmt.Errorf("IAM policy '%s' not found in this account", policyName) + return fmt.Errorf("IAM policy %q not found in this account", policyName) } func (o *initAppOpts) isDomainOwned() error { diff --git a/internal/pkg/cli/app_init_test.go b/internal/pkg/cli/app_init_test.go index 87c35b123e4..b0b9461bcfb 100644 --- a/internal/pkg/cli/app_init_test.go +++ b/internal/pkg/cli/app_init_test.go @@ -95,17 +95,17 @@ func TestInitAppOpts_Validate(t *testing.T) { "wrap error from ListPolicies": { inPBPolicyName: "nonexistentPolicyName", mock: func(m *initAppMocks) { - m.mockPolicyLister.EXPECT().ListPermBoundPolicyNames().Return(nil, errors.New("some error")) + m.mockPolicyLister.EXPECT().ListPolicyNames().Return(nil, errors.New("some error")) }, wantedError: errors.New("list permissions boundary policies: some error"), }, "invalid permissions boundary policy name": { inPBPolicyName: "nonexistentPolicyName", mock: func(m *initAppMocks) { - m.mockPolicyLister.EXPECT().ListPermBoundPolicyNames().Return( + m.mockPolicyLister.EXPECT().ListPolicyNames().Return( []string{"existentPolicyName"}, nil) }, - wantedError: errors.New("IAM policy 'nonexistentPolicyName' not found in this account"), + wantedError: errors.New("IAM policy \"nonexistentPolicyName\" not found in this account"), }, "invalid domain name that doesn't have a hosted zone": { inDomainName: "badMockDomain.com", diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index b34cebf9f57..d0f0a409794 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -556,7 +556,7 @@ type roleDeleter interface { } type policyLister interface { - ListPermBoundPolicyNames() ([]string, error) + ListPolicyNames() ([]string, error) } type serviceDescriber interface { diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index 017b167fe2b..79c895bdca4 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -5914,19 +5914,19 @@ func (m *MockpolicyLister) EXPECT() *MockpolicyListerMockRecorder { return m.recorder } -// ListPermBoundPolicyNames mocks base method. -func (m *MockpolicyLister) ListPermBoundPolicyNames() ([]string, error) { +// ListPolicyNames mocks base method. +func (m *MockpolicyLister) ListPolicyNames() ([]string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ListPermBoundPolicyNames") + ret := m.ctrl.Call(m, "ListPolicyNames") ret0, _ := ret[0].([]string) ret1, _ := ret[1].(error) return ret0, ret1 } -// ListPermBoundPolicyNames indicates an expected call of ListPermBoundPolicyNames. -func (mr *MockpolicyListerMockRecorder) ListPermBoundPolicyNames() *gomock.Call { +// ListPolicyNames indicates an expected call of ListPolicyNames. +func (mr *MockpolicyListerMockRecorder) ListPolicyNames() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListPermBoundPolicyNames", reflect.TypeOf((*MockpolicyLister)(nil).ListPermBoundPolicyNames)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListPolicyNames", reflect.TypeOf((*MockpolicyLister)(nil).ListPolicyNames)) } // MockserviceDescriber is a mock of serviceDescriber interface.