From 40ed8cb38fd3848c71afe223a0a384029c142488 Mon Sep 17 00:00:00 2001 From: Ofir Lapid Date: Fri, 22 Jul 2022 09:27:48 +0300 Subject: [PATCH 01/10] adding region to both ecr_provider and ecr_fetcher + tests --- resources/fetchers/ecr_factory.go | 17 +- resources/fetchers/ecr_factory_test.go | 2 +- resources/fetchers/ecr_fetcher.go | 63 +++-- resources/fetchers/ecr_fetcher_test.go | 232 +++++++++++------- resources/providers/awslib/ecr_provider.go | 40 +-- .../providers/awslib/ecr_provider_mock.go | 86 ------- .../providers/awslib/ecr_public_provider.go | 17 +- .../awslib/ecr_repository_describer_mock.go | 103 ++++++++ 8 files changed, 349 insertions(+), 211 deletions(-) delete mode 100644 resources/providers/awslib/ecr_provider_mock.go create mode 100644 resources/providers/awslib/ecr_repository_describer_mock.go diff --git a/resources/fetchers/ecr_factory.go b/resources/fetchers/ecr_factory.go index cb9a31817e..279b984f23 100644 --- a/resources/fetchers/ecr_factory.go +++ b/resources/fetchers/ecr_factory.go @@ -70,7 +70,7 @@ func (f *ECRFactory) CreateFrom(log *logp.Logger, cfg ECRFetcherConfig, ch chan return nil, fmt.Errorf("failed to initialize AWS credentials: %w", err) } - ecrPrivateProvider := awslib.NewEcrProvider(awsConfig) + ecrPrivateProvider := awslib.NewEcrProvider() ecrPublicProvider := awslib.NewEcrPublicProvider() kubeClient, err := f.KubernetesProvider.GetClient(cfg.KubeConfig, kubernetes.KubeClientOptions{}) if err != nil { @@ -83,15 +83,19 @@ func (f *ECRFactory) CreateFrom(log *logp.Logger, cfg ECRFetcherConfig, ch chan return nil, fmt.Errorf("could not retrieve user identity for ECR fetcher: %w", err) } - privateRepoRegex := fmt.Sprintf(PrivateRepoRegexTemplate, *identity.Account, awsConfig.Region) + privateRepoRegex := fmt.Sprintf(PrivateRepoRegexTemplate, *identity.Account) privateECRExecutor := PodDescriber{ - FilterRegex: regexp.MustCompile(privateRepoRegex), - Provider: ecrPrivateProvider, + FilterRegex: regexp.MustCompile(privateRepoRegex), + Provider: ecrPrivateProvider, + ExtractRegion: ExtractRegionFromEcrImage, + ImageRegexIndex: EcrImageRegexGroup, } publicECRExecutor := PodDescriber{ - FilterRegex: regexp.MustCompile(PublicRepoRegex), - Provider: ecrPublicProvider, + FilterRegex: regexp.MustCompile(PublicRepoRegex), + Provider: ecrPublicProvider, + ExtractRegion: ExtractRegionFromPublicEcrImage, + ImageRegexIndex: PublicEcrImageRegexIndex, } fe := &ECRFetcher{ @@ -103,6 +107,7 @@ func (f *ECRFactory) CreateFrom(log *logp.Logger, cfg ECRFetcherConfig, ch chan publicECRExecutor, }, resourceCh: ch, + awsConfig: awsConfig, } return fe, nil } diff --git a/resources/fetchers/ecr_factory_test.go b/resources/fetchers/ecr_factory_test.go index b2e00ce38a..917667154f 100644 --- a/resources/fetchers/ecr_factory_test.go +++ b/resources/fetchers/ecr_factory_test.go @@ -68,7 +68,7 @@ default_region: us1-east "us1-east", "my-account", []string{ - "^my-account\\.dkr\\.ecr\\.us1-east\\.amazonaws\\.com\\/([-\\w\\.\\/]+)[:,@]?", + "^my-account\\.dkr\\.ecr\\.([-\\w]+)\\.amazonaws\\.com\\/([-\\w\\.\\/]+)[:,@]?", "public\\.ecr\\.aws\\/\\w+\\/([-\\w\\.\\/]+)\\:?", }, }, diff --git a/resources/fetchers/ecr_fetcher.go b/resources/fetchers/ecr_fetcher.go index d7288fbac3..3f29a788cb 100644 --- a/resources/fetchers/ecr_fetcher.go +++ b/resources/fetchers/ecr_fetcher.go @@ -20,21 +20,25 @@ package fetchers import ( "context" "fmt" - "regexp" - - "github.com/elastic/cloudbeat/resources/providers/awslib" - v1 "k8s.io/api/core/v1" - + "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/ecr" "github.com/elastic/cloudbeat/resources/fetching" + "github.com/elastic/cloudbeat/resources/providers/awslib" "github.com/elastic/elastic-agent-libs/logp" "github.com/gofrs/uuid" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8s "k8s.io/client-go/kubernetes" + "regexp" ) -const PrivateRepoRegexTemplate = "^%s\\.dkr\\.ecr\\.%s\\.amazonaws\\.com\\/([-\\w\\.\\/]+)[:,@]?" -const PublicRepoRegex = "public\\.ecr\\.aws\\/\\w+\\/([-\\w\\.\\/]+)\\:?" +const ( + PrivateRepoRegexTemplate = "^%s\\.dkr\\.ecr\\.([-\\w]+)\\.amazonaws\\.com\\/([-\\w\\.\\/]+)[:,@]?" + PublicRepoRegex = "public\\.ecr\\.aws\\/\\w+\\/([-\\w\\.\\/]+)\\:?" + EcrRegionRegexGroup = 1 + PublicEcrImageRegexIndex = 1 + EcrImageRegexGroup = 2 +) type ECRFetcher struct { log *logp.Logger @@ -42,11 +46,14 @@ type ECRFetcher struct { kubeClient k8s.Interface PodDescribers []PodDescriber resourceCh chan fetching.ResourceInfo + awsConfig aws.Config } type PodDescriber struct { - FilterRegex *regexp.Regexp - Provider awslib.EcrRepositoryDescriber + FilterRegex *regexp.Regexp + Provider awslib.EcrRepositoryDescriber + ExtractRegion func(describer PodDescriber, repo string) string + ImageRegexIndex int } type ECRFetcherConfig struct { @@ -84,13 +91,26 @@ func (f *ECRFetcher) Fetch(ctx context.Context, cMetadata fetching.CycleMetadata } } } - return nil } func (f *ECRFetcher) describePodImagesRepositories(ctx context.Context, podsList *v1.PodList, describer PodDescriber) ([]ecr.Repository, error) { + regionToReposMap := getAwsRepositories(podsList, describer) + f.log.Debugf("sending pods to ecrProviders: %v", regionToReposMap) + awsRepositories := make([]ecr.Repository, 0) + for region, repositories := range regionToReposMap { + // Add configuration + describedRepo, err := describer.Provider.DescribeRepositories(ctx, f.awsConfig, repositories, region) + if err != nil { + f.log.Errorf("could not retrieve pod's aws repositories for region %s: %w", region, err) + } + awsRepositories = append(awsRepositories, describedRepo.Repositories...) + } + return awsRepositories, nil +} - repositories := make([]string, 0) +func getAwsRepositories(podsList *v1.PodList, describer PodDescriber) map[string][]string { + reposByRegion := make(map[string][]string, 0) for _, pod := range podsList.Items { for _, container := range pod.Spec.Containers { @@ -98,15 +118,28 @@ func (f *ECRFetcher) describePodImagesRepositories(ctx context.Context, podsList // Takes only aws images regexMatcher := describer.FilterRegex.FindStringSubmatch(image) if regexMatcher != nil { - repository := regexMatcher[1] - repositories = append(repositories, repository) + repository := regexMatcher[describer.ImageRegexIndex] + region := describer.ExtractRegion(describer, image) + reposByRegion[region] = append(reposByRegion[region], repository) } } } + return reposByRegion +} + +func ExtractRegionFromEcrImage(describer PodDescriber, image string) string { + regexMatcher := describer.FilterRegex.FindStringSubmatch(image) + if regexMatcher != nil { + repository := regexMatcher[EcrRegionRegexGroup] + return repository + } - f.log.Debugf("sending pods to ecrProviders: %v", repositories) + return "" +} - return describer.Provider.DescribeRepositories(ctx, repositories) +// ExtractRegionFromPublicEcrImage TODO Ofir - https://github.com/elastic/security-team/issues/4035 +func ExtractRegionFromPublicEcrImage(_ PodDescriber, _ string) string { + return "" } func (res ECRResource) GetData() interface{} { diff --git a/resources/fetchers/ecr_fetcher_test.go b/resources/fetchers/ecr_fetcher_test.go index bea2150887..fa2cfbc3f8 100644 --- a/resources/fetchers/ecr_fetcher_test.go +++ b/resources/fetchers/ecr_fetcher_test.go @@ -20,6 +20,7 @@ package fetchers import ( "context" "fmt" + "github.com/aws/aws-sdk-go-v2/aws" "github.com/elastic/cloudbeat/resources/fetching" "regexp" "testing" @@ -43,6 +44,11 @@ type ECRFetcherTestSuite struct { resourceCh chan fetching.ResourceInfo } +type describeRepoMockParameters struct { + Repositories []ecr.Repository + ExpectedImages []string +} + func TestECRFetcherTestSuite(t *testing.T) { s := new(ECRFetcherTestSuite) s.log = logp.NewLogger("cloudbeat_ecr_fetcher_test_suite") @@ -64,22 +70,19 @@ func (s *ECRFetcherTestSuite) TearDownTest() { func (s *ECRFetcherTestSuite) TestCreateFetcher() { firstRepositoryName := "cloudbeat" secondRepositoryName := "cloudbeat1" - publicRepoName := "build.security/citools" privateRepoWithSlash := "build/cloudbeat" + publicRepoName := "build.security/citools" var tests = []struct { - identityAccount string - region string - namespace string - containers []v1.Container - expectedRepositories []ecr.Repository - expectedPublicRepositories []ecr.Repository - expectedRepositoriesNames []string - expectedPublicRepositoriesNames []string + identityAccount string + namespace string + containers []v1.Container + privateRepositoriesResponseByRegion map[string]describeRepoMockParameters + publicRepositoriesResponseByRegion map[string]describeRepoMockParameters + expectedRepositoriesNames []string }{ { "123456789123", - "us-east-2", "my-namespace", []v1.Container{ { @@ -91,21 +94,26 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { Name: "cloudbeat1", }, }, - []ecr.Repository{{ - ImageScanningConfiguration: nil, - RepositoryName: &firstRepositoryName, - RepositoryUri: nil, - }, { - ImageScanningConfiguration: nil, - RepositoryName: &secondRepositoryName, - RepositoryUri: nil, - }}, - []ecr.Repository{}, + map[string]describeRepoMockParameters{ + "us-east-2": { + ExpectedImages: []string{"cloudbeat", "cloudbeat1"}, + Repositories: []ecr.Repository{ + { + ImageScanningConfiguration: nil, + RepositoryName: &firstRepositoryName, + RepositoryUri: nil, + }, { + ImageScanningConfiguration: nil, + RepositoryName: &secondRepositoryName, + RepositoryUri: nil, + }}, + }, + }, + map[string]describeRepoMockParameters{}, []string{firstRepositoryName, secondRepositoryName}, - []string{}, - }, { + }, + { "123456789123", - "us-east-2", "my-namespace", []v1.Container{ { @@ -117,22 +125,25 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { Name: "cloudbeat1", }, }, - []ecr.Repository{{ - ImageScanningConfiguration: nil, - RepositoryName: &privateRepoWithSlash, - RepositoryUri: nil, - }, { - ImageScanningConfiguration: nil, - RepositoryName: &secondRepositoryName, - RepositoryUri: nil, - }}, - []ecr.Repository{}, + map[string]describeRepoMockParameters{ + "us-east-2": { + ExpectedImages: []string{"build/cloudbeat", "cloudbeat1"}, + Repositories: []ecr.Repository{ + { + ImageScanningConfiguration: nil, + RepositoryName: &privateRepoWithSlash, + RepositoryUri: nil, + }, { + ImageScanningConfiguration: nil, + RepositoryName: &secondRepositoryName, + RepositoryUri: nil, + }}, + }, + }, map[string]describeRepoMockParameters{}, []string{privateRepoWithSlash, secondRepositoryName}, - []string{}, }, { "123456789123", - "us-east-2", "my-namespace", []v1.Container{ { @@ -144,21 +155,32 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { Name: "build.security/citools", }, }, - []ecr.Repository{{ - ImageScanningConfiguration: nil, - RepositoryName: &firstRepositoryName, - RepositoryUri: nil, - }}, []ecr.Repository{{ - ImageScanningConfiguration: nil, - RepositoryName: &publicRepoName, - RepositoryUri: nil, - }}, - []string{firstRepositoryName}, - []string{publicRepoName}, + map[string]describeRepoMockParameters{ + "us-east-2": { + ExpectedImages: []string{"cloudbeat"}, + Repositories: []ecr.Repository{ + { + ImageScanningConfiguration: nil, + RepositoryName: &firstRepositoryName, + RepositoryUri: nil, + }}, + }, + }, + map[string]describeRepoMockParameters{ + "": { + ExpectedImages: []string{"build.security/citools"}, + Repositories: []ecr.Repository{ + { + ImageScanningConfiguration: nil, + RepositoryName: &publicRepoName, + RepositoryUri: nil, + }}, + }, + }, + []string{firstRepositoryName, publicRepoName}, }, { "123456789123", - "us-east-2", "my-namespace", []v1.Container{ { @@ -170,29 +192,45 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { Name: "cloudbeat1", }, }, - []ecr.Repository{}, - []ecr.Repository{}, - []string{}, + map[string]describeRepoMockParameters{}, + map[string]describeRepoMockParameters{}, []string{}, }, { "123456789123", - "us-east-1", "my-namespace", []v1.Container{ { - Image: "123456789123.dkr.ecr.wrong-region.amazonaws.com/cloudbeat:latest", + Image: "123456789123.dkr.ecr.us-east-2.amazonaws.com/cloudbeat:latest", Name: "cloudbeat", }, { - Image: "123456789123.dkr.ecr.wrong-region.amazonaws.com/cloudbeat1:latest", + Image: "123456789123.dkr.ecr.us-east-1.amazonaws.com/cloudbeat1:latest", Name: "cloudbeat1", }, }, - []ecr.Repository{}, - []ecr.Repository{}, - []string{}, - []string{}, + map[string]describeRepoMockParameters{ + "us-east-2": { + ExpectedImages: []string{"cloudbeat"}, + Repositories: []ecr.Repository{ + { + ImageScanningConfiguration: nil, + RepositoryName: &firstRepositoryName, + RepositoryUri: nil, + }}, + }, + "us-east-1": { + ExpectedImages: []string{"cloudbeat1"}, + Repositories: []ecr.Repository{ + { + ImageScanningConfiguration: nil, + RepositoryName: &secondRepositoryName, + RepositoryUri: nil, + }}, + }, + }, + map[string]describeRepoMockParameters{}, + []string{firstRepositoryName, secondRepositoryName}, }, } for _, test := range tests { @@ -219,30 +257,55 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { mockedKubernetesClientGetter := &providers.MockedKubernetesClientGetter{} mockedKubernetesClientGetter.EXPECT().GetClient(mock.Anything, mock.Anything).Return(kubeclient, nil) - // Needs to use the same services - ecrProvider := &awslib.MockedEcrRepositoryDescriber{} - ecrProvider.EXPECT().DescribeRepositories(mock.Anything, mock.MatchedBy(func(repo []string) bool { - return s.Equal(test.expectedRepositoriesNames, repo) - })).Return(test.expectedRepositories, nil) + ecrProvider := &awslib.MockEcrRepositoryDescriber{} + // Init private repositories provider - ecrPublicProvider := &awslib.MockedEcrRepositoryDescriber{} - ecrPublicProvider.EXPECT().DescribeRepositories(mock.Anything, mock.MatchedBy(func(repo []string) bool { - return s.Equal(test.expectedPublicRepositoriesNames, repo) - })).Return(test.expectedPublicRepositories, nil) + ecrProvider.EXPECT().DescribeRepositories(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Call. + Return(func(ctx context.Context, cfg aws.Config, repoNames []string, region string) awslib.ECRProviderResponse { + response, ok := test.privateRepositoriesResponseByRegion[region] + s.True(ok) + s.Equal(response.ExpectedImages, repoNames) + + return awslib.ECRProviderResponse{ + Repositories: response.Repositories, + } + }, + func(ctx context.Context, cfg aws.Config, repoNames []string, region string) error { + return nil + }) + + // Init public repositories provider + ecrPublicProvider := &awslib.MockEcrRepositoryDescriber{} + ecrPublicProvider.EXPECT().DescribeRepositories(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Call. + Return(func(ctx context.Context, cfg aws.Config, repoNames []string, region string) awslib.ECRProviderResponse { + response, ok := test.publicRepositoriesResponseByRegion[region] + s.True(ok) + s.Equal(response.ExpectedImages, repoNames) + + return awslib.ECRProviderResponse{ + Repositories: response.Repositories, + } + }, + func(ctx context.Context, cfg aws.Config, repoNames []string, region string) error { + return nil + }) - privateRepoRegex := fmt.Sprintf(PrivateRepoRegexTemplate, test.identityAccount, test.region) + privateRepoRegex := fmt.Sprintf(PrivateRepoRegexTemplate, test.identityAccount) privateEcrExecutor := PodDescriber{ - FilterRegex: regexp.MustCompile(privateRepoRegex), - Provider: ecrProvider, + FilterRegex: regexp.MustCompile(privateRepoRegex), + Provider: ecrProvider, + ExtractRegion: ExtractRegionFromEcrImage, + ImageRegexIndex: EcrImageRegexGroup, } publicEcrExecutor := PodDescriber{ - FilterRegex: regexp.MustCompile(PublicRepoRegex), - Provider: ecrPublicProvider, + FilterRegex: regexp.MustCompile(PublicRepoRegex), + Provider: ecrPublicProvider, + ExtractRegion: ExtractRegionFromPublicEcrImage, + ImageRegexIndex: PublicEcrImageRegexIndex, } - expectedRepositories := append(test.expectedRepositories, test.expectedPublicRepositories...) ecrFetcher := ECRFetcher{ log: s.log, cfg: ECRFetcherConfig{}, @@ -255,7 +318,7 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { err = ecrFetcher.Fetch(ctx, fetching.CycleMetadata{}) results := testhelper.CollectResources(s.resourceCh) - s.Equal(len(expectedRepositories), len(results)) + s.Equal(len(test.expectedRepositoriesNames), len(results)) s.NoError(err) for i, name := range test.expectedRepositoriesNames { @@ -312,22 +375,26 @@ func (s *ECRFetcherTestSuite) TestCreateFetcherErrorCases() { mockedKubernetesClientGetter.EXPECT().GetClient(mock.Anything, mock.Anything).Return(kubeclient, nil) // Needs to use the same services - ecrProvider := &awslib.MockedEcrRepositoryDescriber{} - ecrProvider.EXPECT().DescribeRepositories(mock.Anything, mock.Anything).Return(nil, test.error) + ecrProvider := &awslib.MockEcrRepositoryDescriber{} + ecrProvider.EXPECT().DescribeRepositories(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(awslib.ECRProviderResponse{}, test.error) - ecrPublicProvider := &awslib.MockedEcrRepositoryDescriber{} - ecrPublicProvider.EXPECT().DescribeRepositories(mock.Anything, mock.Anything).Return(nil, test.error) + ecrPublicProvider := &awslib.MockEcrRepositoryDescriber{} + ecrPublicProvider.EXPECT().DescribeRepositories(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(awslib.ECRProviderResponse{}, test.error) - privateRepoRegex := fmt.Sprintf(PrivateRepoRegexTemplate, test.identityAccount, test.region) + privateRepoRegex := fmt.Sprintf(PrivateRepoRegexTemplate, test.identityAccount) privateEcrExecutor := PodDescriber{ - FilterRegex: regexp.MustCompile(privateRepoRegex), - Provider: ecrProvider, + FilterRegex: regexp.MustCompile(privateRepoRegex), + Provider: ecrProvider, + ExtractRegion: ExtractRegionFromEcrImage, + ImageRegexIndex: EcrImageRegexGroup, } publicEcrExecutor := PodDescriber{ - FilterRegex: regexp.MustCompile(PublicRepoRegex), - Provider: ecrPublicProvider, + FilterRegex: regexp.MustCompile(PublicRepoRegex), + Provider: ecrPublicProvider, + ExtractRegion: ExtractRegionFromPublicEcrImage, + ImageRegexIndex: PublicEcrImageRegexIndex, } ecrFetcher := ECRFetcher{ @@ -343,6 +410,5 @@ func (s *ECRFetcherTestSuite) TestCreateFetcherErrorCases() { results := testhelper.CollectResources(s.resourceCh) s.Equal(0, len(results)) - s.EqualError(err, "could not retrieve pod's aws repositories: ecr error") } } diff --git a/resources/providers/awslib/ecr_provider.go b/resources/providers/awslib/ecr_provider.go index edf8d69736..d9e48a2c51 100644 --- a/resources/providers/awslib/ecr_provider.go +++ b/resources/providers/awslib/ecr_provider.go @@ -29,36 +29,48 @@ type ECRProvider struct { client *ecr.Client } +type ECRProviderResponse struct { + Repositories []ecr.Repository + PaginationToken string +} + type EcrRepositoryDescriber interface { - DescribeRepositories(ctx context.Context, repoNames []string) ([]ecr.Repository, error) + DescribeRepositories(ctx context.Context, cfg aws.Config, repoNames []string, region string) (ECRProviderResponse, error) } -func NewEcrProvider(cfg aws.Config) *ECRProvider { - svc := ecr.New(cfg) - return &ECRProvider{ - client: svc, - } +func NewEcrProvider() *ECRProvider { + return &ECRProvider{} } // DescribeAllECRRepositories This method will return a maximum of 100 repository /// If we will ever wish to change it, DescribeRepositories returns results in paginated manner -func (provider *ECRProvider) DescribeAllECRRepositories(ctx context.Context) ([]ecr.Repository, error) { - /// When repoNames is nil, it will describe all the existing repositories - return provider.DescribeRepositories(ctx, nil) +func (provider *ECRProvider) DescribeAllECRRepositories(ctx context.Context, cfg aws.Config, region string) (ECRProviderResponse, error) { + /// When repoNames is nil, it will describe all the existing Repositories + return provider.DescribeRepositories(ctx, cfg, nil, region) } // DescribeRepositories This method will return a maximum of 100 repository /// If we will ever wish to change it, DescribeRepositories returns results in paginated manner -/// When repoNames is nil, it will describe all the existing repositories -func (provider *ECRProvider) DescribeRepositories(ctx context.Context, repoNames []string) ([]ecr.Repository, error) { +/// When repoNames is nil, it will describe all the existing Repositories +func (provider *ECRProvider) DescribeRepositories(ctx context.Context, cfg aws.Config, repoNames []string, region string) (ECRProviderResponse, error) { + if len(repoNames) == 0 { + return ECRProviderResponse{}, nil + } + + cfg.Region = region + svc := ecr.New(cfg) input := &ecr.DescribeRepositoriesInput{ RepositoryNames: repoNames, } - req := provider.client.DescribeRepositoriesRequest(input) + req := svc.DescribeRepositoriesRequest(input) + response, err := req.Send(ctx) if err != nil { - return nil, fmt.Errorf("failed to fetch repository:%s from ecr, error - %w", repoNames, err) + return ECRProviderResponse{}, fmt.Errorf("failed to fetch repository:%s from ecr, error - %w", repoNames, err) } - return response.Repositories, err + return ECRProviderResponse{ + Repositories: response.Repositories, + PaginationToken: *response.NextToken, + }, err } diff --git a/resources/providers/awslib/ecr_provider_mock.go b/resources/providers/awslib/ecr_provider_mock.go deleted file mode 100644 index 57bd24421f..0000000000 --- a/resources/providers/awslib/ecr_provider_mock.go +++ /dev/null @@ -1,86 +0,0 @@ -// Licensed to Elasticsearch B.V. under one or more contributor -// license agreements. See the NOTICE file distributed with -// this work for additional information regarding copyright -// ownership. Elasticsearch B.V. licenses this file to you under -// the Apache License, Version 2.0 (the "License"); you may -// not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. -// Code generated by mockery v2.10.4. DO NOT EDIT. - -package awslib - -import ( - context "context" - - ecr "github.com/aws/aws-sdk-go-v2/service/ecr" - mock "github.com/stretchr/testify/mock" -) - -// MockedEcrRepositoryDescriber is an autogenerated mock type for the EcrRepositoryDescriber type -type MockedEcrRepositoryDescriber struct { - mock.Mock -} - -type MockEcrRepositoryDescriber_Expecter struct { - mock *mock.Mock -} - -func (_m *MockedEcrRepositoryDescriber) EXPECT() *MockEcrRepositoryDescriber_Expecter { - return &MockEcrRepositoryDescriber_Expecter{mock: &_m.Mock} -} - -// DescribeRepositories provides a mock function with given fields: ctx, repoNames -func (_m *MockedEcrRepositoryDescriber) DescribeRepositories(ctx context.Context, repoNames []string) ([]ecr.Repository, error) { - ret := _m.Called(ctx, repoNames) - - var r0 []ecr.Repository - if rf, ok := ret.Get(0).(func(context.Context, []string) []ecr.Repository); ok { - r0 = rf(ctx, repoNames) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]ecr.Repository) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func(context.Context, []string) error); ok { - r1 = rf(ctx, repoNames) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// MockEcrRepositoryDescriber_DescribeRepositories_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DescribeRepositories' -type MockEcrRepositoryDescriber_DescribeRepositories_Call struct { - *mock.Call -} - -// DescribeRepositories is a helper method to define mock.On call -// - ctx context.Context -// - repoNames []string -func (_e *MockEcrRepositoryDescriber_Expecter) DescribeRepositories(ctx interface{}, repoNames interface{}) *MockEcrRepositoryDescriber_DescribeRepositories_Call { - return &MockEcrRepositoryDescriber_DescribeRepositories_Call{Call: _e.mock.On("DescribeRepositories", ctx, repoNames)} -} - -func (_c *MockEcrRepositoryDescriber_DescribeRepositories_Call) Run(run func(ctx context.Context, repoNames []string)) *MockEcrRepositoryDescriber_DescribeRepositories_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].([]string)) - }) - return _c -} - -func (_c *MockEcrRepositoryDescriber_DescribeRepositories_Call) Return(_a0 []ecr.Repository, _a1 error) *MockEcrRepositoryDescriber_DescribeRepositories_Call { - _c.Call.Return(_a0, _a1) - return _c -} diff --git a/resources/providers/awslib/ecr_public_provider.go b/resources/providers/awslib/ecr_public_provider.go index 5707b87ea6..d1e4ee3133 100644 --- a/resources/providers/awslib/ecr_public_provider.go +++ b/resources/providers/awslib/ecr_public_provider.go @@ -19,6 +19,7 @@ package awslib import ( "context" + "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/ecr" ) @@ -34,16 +35,20 @@ func NewEcrPublicProvider() *ECRPublicProvider { // DescribeAllECRRepositories This method will return a maximum of 100 repository /// If we will ever wish to change it, DescribeRepositories returns results in paginated manner -func (provider *ECRPublicProvider) DescribeAllECRRepositories(ctx context.Context) ([]ecr.Repository, error) { - /// When repoNames is nil, it will describe all the existing repositories +func (provider *ECRPublicProvider) DescribeAllECRRepositories(_ context.Context, _ aws.Config, _ string) (ECRProviderResponse, error) { + /// When repoNames is nil, it will describe all the existing Repositories var emptyArray = make([]ecr.Repository, 0) - return emptyArray, nil + return ECRProviderResponse{ + Repositories: emptyArray, + }, nil } // DescribeRepositories This method will return a maximum of 100 repository /// If we will ever wish to change it, DescribeRepositories returns results in paginated manner -/// When repoNames is nil, it will describe all the existing repositories -func (provider *ECRPublicProvider) DescribeRepositories(ctx context.Context, repoNames []string) ([]ecr.Repository, error) { +/// When repoNames is nil, it will describe all the existing Repositories +func (provider *ECRPublicProvider) DescribeRepositories(_ context.Context, _ aws.Config, _ []string, _ string) (ECRProviderResponse, error) { var emptyArray = make([]ecr.Repository, 0) - return emptyArray, nil + return ECRProviderResponse{ + Repositories: emptyArray, + }, nil } diff --git a/resources/providers/awslib/ecr_repository_describer_mock.go b/resources/providers/awslib/ecr_repository_describer_mock.go new file mode 100644 index 0000000000..46240a29cb --- /dev/null +++ b/resources/providers/awslib/ecr_repository_describer_mock.go @@ -0,0 +1,103 @@ +//// Licensed to Elasticsearch B.V. under one or more contributor +//// license agreements. See the NOTICE file distributed with +//// this work for additional information regarding copyright +//// ownership. Elasticsearch B.V. licenses this file to you under +//// the Apache License, Version 2.0 (the "License"); you may +//// not use this file except in compliance with the License. +//// You may obtain a copy of the License at +//// +//// http://www.apache.org/licenses/LICENSE-2.0 +//// +//// Unless required by applicable law or agreed to in writing, +//// software distributed under the License is distributed on an +//// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +//// KIND, either express or implied. See the License for the +//// specific language governing permissions and limitations +//// under the License. +// +// Code generated by mockery v2.14.0. DO NOT EDIT. + +package awslib + +import ( + context "context" + + aws "github.com/aws/aws-sdk-go-v2/aws" + + mock "github.com/stretchr/testify/mock" +) + +// MockEcrRepositoryDescriber is an autogenerated mock type for the EcrRepositoryDescriber type +type MockEcrRepositoryDescriber struct { + mock.Mock +} + +type MockEcrRepositoryDescriber_Expecter struct { + mock *mock.Mock +} + +func (_m *MockEcrRepositoryDescriber) EXPECT() *MockEcrRepositoryDescriber_Expecter { + return &MockEcrRepositoryDescriber_Expecter{mock: &_m.Mock} +} + +// DescribeRepositories provides a mock function with given fields: ctx, cfg, repoNames, region +func (_m *MockEcrRepositoryDescriber) DescribeRepositories(ctx context.Context, cfg aws.Config, repoNames []string, region string) (ECRProviderResponse, error) { + ret := _m.Called(ctx, cfg, repoNames, region) + + var r0 ECRProviderResponse + if rf, ok := ret.Get(0).(func(context.Context, aws.Config, []string, string) ECRProviderResponse); ok { + r0 = rf(ctx, cfg, repoNames, region) + } else { + r0 = ret.Get(0).(ECRProviderResponse) + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, aws.Config, []string, string) error); ok { + r1 = rf(ctx, cfg, repoNames, region) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockEcrRepositoryDescriber_DescribeRepositories_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DescribeRepositories' +type MockEcrRepositoryDescriber_DescribeRepositories_Call struct { + *mock.Call +} + +// DescribeRepositories is a helper method to define mock.On call +// - ctx context.Context +// - cfg aws.Config +// - repoNames []string +// - region string +func (_e *MockEcrRepositoryDescriber_Expecter) DescribeRepositories(ctx interface{}, cfg interface{}, repoNames interface{}, region interface{}) *MockEcrRepositoryDescriber_DescribeRepositories_Call { + return &MockEcrRepositoryDescriber_DescribeRepositories_Call{Call: _e.mock.On("DescribeRepositories", ctx, cfg, repoNames, region)} +} + +func (_c *MockEcrRepositoryDescriber_DescribeRepositories_Call) Run(run func(ctx context.Context, cfg aws.Config, repoNames []string, region string)) *MockEcrRepositoryDescriber_DescribeRepositories_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(aws.Config), args[2].([]string), args[3].(string)) + }) + return _c +} + +func (_c *MockEcrRepositoryDescriber_DescribeRepositories_Call) Return(_a0 ECRProviderResponse, _a1 error) *MockEcrRepositoryDescriber_DescribeRepositories_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +type mockConstructorTestingTNewMockEcrRepositoryDescriber interface { + mock.TestingT + Cleanup(func()) +} + +// NewMockEcrRepositoryDescriber creates a new instance of MockEcrRepositoryDescriber. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewMockEcrRepositoryDescriber(t mockConstructorTestingTNewMockEcrRepositoryDescriber) *MockEcrRepositoryDescriber { + mock := &MockEcrRepositoryDescriber{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} From 3f2bc711e41e022cf28cdd302241c83cc511f5b8 Mon Sep 17 00:00:00 2001 From: Ofir Lapid Date: Fri, 22 Jul 2022 09:43:56 +0300 Subject: [PATCH 02/10] Fixing test --- resources/fetchers/ecr_fetcher_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/resources/fetchers/ecr_fetcher_test.go b/resources/fetchers/ecr_fetcher_test.go index 46072612d1..6b59191f86 100644 --- a/resources/fetchers/ecr_fetcher_test.go +++ b/resources/fetchers/ecr_fetcher_test.go @@ -222,6 +222,7 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { ExpectedImages: []string{"cloudbeat"}, Repositories: []ecr.Repository{ { + RepositoryArn: &repoArn, ImageScanningConfiguration: nil, RepositoryName: &firstRepositoryName, RepositoryUri: nil, @@ -231,6 +232,7 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { ExpectedImages: []string{"cloudbeat1"}, Repositories: []ecr.Repository{ { + RepositoryArn: &repoArn, ImageScanningConfiguration: nil, RepositoryName: &secondRepositoryName, RepositoryUri: nil, From dca0d80bf9092e2421ca0939a03f8dd1e08dc169 Mon Sep 17 00:00:00 2001 From: Ofir Lapid Date: Fri, 22 Jul 2022 10:01:47 +0300 Subject: [PATCH 03/10] Self review --- resources/fetchers/ecr_factory_test.go | 5 +++ resources/fetchers/ecr_fetcher_test.go | 39 +++++++++++----------- resources/providers/awslib/ecr_provider.go | 1 - 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/resources/fetchers/ecr_factory_test.go b/resources/fetchers/ecr_factory_test.go index 917667154f..d94ae96a3b 100644 --- a/resources/fetchers/ecr_factory_test.go +++ b/resources/fetchers/ecr_factory_test.go @@ -113,10 +113,15 @@ default_region: us1-east s.NotNil(fetcher) ecrFetcher, ok := fetcher.(*ECRFetcher) + expectedEcrImageRegexIndex := 2 + expectedEcrPublicImageRegexIndex := 1 + s.True(ok) s.Equal(kubeclient, ecrFetcher.kubeClient) s.Equal(test.expectedRegex[0], ecrFetcher.PodDescribers[0].FilterRegex.String()) + s.Equal(expectedEcrImageRegexIndex, ecrFetcher.PodDescribers[0].ImageRegexIndex) s.Equal(test.expectedRegex[1], ecrFetcher.PodDescribers[1].FilterRegex.String()) + s.Equal(expectedEcrPublicImageRegexIndex, ecrFetcher.PodDescribers[1].ImageRegexIndex) } } diff --git a/resources/fetchers/ecr_fetcher_test.go b/resources/fetchers/ecr_fetcher_test.go index 6b59191f86..ce33f4901a 100644 --- a/resources/fetchers/ecr_fetcher_test.go +++ b/resources/fetchers/ecr_fetcher_test.go @@ -45,8 +45,8 @@ type ECRFetcherTestSuite struct { } type describeRepoMockParameters struct { - Repositories []ecr.Repository - ExpectedImages []string + EcrRepositories []ecr.Repository + ExpectedRepositories []string } func TestECRFetcherTestSuite(t *testing.T) { @@ -97,8 +97,8 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { }, map[string]describeRepoMockParameters{ "us-east-2": { - ExpectedImages: []string{"cloudbeat", "cloudbeat1"}, - Repositories: []ecr.Repository{ + ExpectedRepositories: []string{"cloudbeat", "cloudbeat1"}, + EcrRepositories: []ecr.Repository{ { RepositoryArn: &repoArn, ImageScanningConfiguration: nil, @@ -131,8 +131,8 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { }, map[string]describeRepoMockParameters{ "us-east-2": { - ExpectedImages: []string{"build/cloudbeat", "cloudbeat1"}, - Repositories: []ecr.Repository{ + ExpectedRepositories: []string{"build/cloudbeat", "cloudbeat1"}, + EcrRepositories: []ecr.Repository{ { RepositoryArn: &repoArn, ImageScanningConfiguration: nil, @@ -163,8 +163,8 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { }, map[string]describeRepoMockParameters{ "us-east-2": { - ExpectedImages: []string{"cloudbeat"}, - Repositories: []ecr.Repository{ + ExpectedRepositories: []string{"cloudbeat"}, + EcrRepositories: []ecr.Repository{ { RepositoryArn: &repoArn, ImageScanningConfiguration: nil, @@ -175,8 +175,8 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { }, map[string]describeRepoMockParameters{ "": { - ExpectedImages: []string{"build.security/citools"}, - Repositories: []ecr.Repository{ + ExpectedRepositories: []string{"build.security/citools"}, + EcrRepositories: []ecr.Repository{ { RepositoryArn: &repoArn, ImageScanningConfiguration: nil, @@ -219,8 +219,8 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { }, map[string]describeRepoMockParameters{ "us-east-2": { - ExpectedImages: []string{"cloudbeat"}, - Repositories: []ecr.Repository{ + ExpectedRepositories: []string{"cloudbeat"}, + EcrRepositories: []ecr.Repository{ { RepositoryArn: &repoArn, ImageScanningConfiguration: nil, @@ -229,8 +229,8 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { }}, }, "us-east-1": { - ExpectedImages: []string{"cloudbeat1"}, - Repositories: []ecr.Repository{ + ExpectedRepositories: []string{"cloudbeat1"}, + EcrRepositories: []ecr.Repository{ { RepositoryArn: &repoArn, ImageScanningConfiguration: nil, @@ -274,10 +274,10 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { Return(func(ctx context.Context, cfg aws.Config, repoNames []string, region string) awslib.ECRProviderResponse { response, ok := test.privateRepositoriesResponseByRegion[region] s.True(ok) - s.Equal(response.ExpectedImages, repoNames) + s.Equal(response.ExpectedRepositories, repoNames) return awslib.ECRProviderResponse{ - Repositories: response.Repositories, + Repositories: response.EcrRepositories, } }, func(ctx context.Context, cfg aws.Config, repoNames []string, region string) error { @@ -290,10 +290,10 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { Return(func(ctx context.Context, cfg aws.Config, repoNames []string, region string) awslib.ECRProviderResponse { response, ok := test.publicRepositoriesResponseByRegion[region] s.True(ok) - s.Equal(response.ExpectedImages, repoNames) + s.Equal(response.ExpectedRepositories, repoNames) return awslib.ECRProviderResponse{ - Repositories: response.Repositories, + Repositories: response.EcrRepositories, } }, func(ctx context.Context, cfg aws.Config, repoNames []string, region string) error { @@ -388,7 +388,6 @@ func (s *ECRFetcherTestSuite) TestCreateFetcherErrorCases() { mockedKubernetesClientGetter := &providers.MockedKubernetesClientGetter{} mockedKubernetesClientGetter.EXPECT().GetClient(mock.Anything, mock.Anything).Return(kubeclient, nil) - // Needs to use the same services ecrProvider := &awslib.MockEcrRepositoryDescriber{} ecrProvider.EXPECT().DescribeRepositories(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(awslib.ECRProviderResponse{}, test.error) @@ -421,7 +420,7 @@ func (s *ECRFetcherTestSuite) TestCreateFetcherErrorCases() { ctx := context.Background() err = ecrFetcher.Fetch(ctx, fetching.CycleMetadata{}) - + s.Nil(err) results := testhelper.CollectResources(s.resourceCh) s.Equal(0, len(results)) } diff --git a/resources/providers/awslib/ecr_provider.go b/resources/providers/awslib/ecr_provider.go index d9e48a2c51..a588ab08b8 100644 --- a/resources/providers/awslib/ecr_provider.go +++ b/resources/providers/awslib/ecr_provider.go @@ -26,7 +26,6 @@ import ( ) type ECRProvider struct { - client *ecr.Client } type ECRProviderResponse struct { From b502d7833a66897fee7d2226611d70c2f945f13f Mon Sep 17 00:00:00 2001 From: Ofir Lapid Date: Fri, 22 Jul 2022 10:25:22 +0300 Subject: [PATCH 04/10] Add license to ecr_repository_describer_mock.go --- .../awslib/ecr_repository_describer_mock.go | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/resources/providers/awslib/ecr_repository_describer_mock.go b/resources/providers/awslib/ecr_repository_describer_mock.go index 46240a29cb..7707da9ff6 100644 --- a/resources/providers/awslib/ecr_repository_describer_mock.go +++ b/resources/providers/awslib/ecr_repository_describer_mock.go @@ -1,21 +1,20 @@ -//// Licensed to Elasticsearch B.V. under one or more contributor -//// license agreements. See the NOTICE file distributed with -//// this work for additional information regarding copyright -//// ownership. Elasticsearch B.V. licenses this file to you under -//// the Apache License, Version 2.0 (the "License"); you may -//// not use this file except in compliance with the License. -//// You may obtain a copy of the License at -//// -//// http://www.apache.org/licenses/LICENSE-2.0 -//// -//// Unless required by applicable law or agreed to in writing, -//// software distributed under the License is distributed on an -//// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -//// KIND, either express or implied. See the License for the -//// specific language governing permissions and limitations -//// under the License. +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at // -// Code generated by mockery v2.14.0. DO NOT EDIT. +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// Code generated by mockery v2.10.4. DO NOT EDIT. package awslib From 5237e893404f3f1c1c3aa0e7a37dfaf6726618a9 Mon Sep 17 00:00:00 2001 From: Ofir Lapid Date: Sun, 24 Jul 2022 19:12:23 +0300 Subject: [PATCH 05/10] Adding bundle.tag.gz to gitignore --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 53a9e9218f..083adf1928 100644 --- a/.gitignore +++ b/.gitignore @@ -33,4 +33,5 @@ tests/allure/reports/* # vscode .vscode* -*.code-workspace \ No newline at end of file +*.code-workspace +/evaluator/tmpBundles/bundle.tar.gz From 92c287d1e644c0960219b0b01e823c92118da97b Mon Sep 17 00:00:00 2001 From: ofiriro3 Date: Sun, 24 Jul 2022 19:12:47 +0300 Subject: [PATCH 06/10] Update resources/fetchers/ecr_fetcher_test.go Co-authored-by: Oren Zohar <85433724+oren-zohar@users.noreply.github.com> --- resources/fetchers/ecr_fetcher_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/fetchers/ecr_fetcher_test.go b/resources/fetchers/ecr_fetcher_test.go index ce33f4901a..fb1f9f7ed9 100644 --- a/resources/fetchers/ecr_fetcher_test.go +++ b/resources/fetchers/ecr_fetcher_test.go @@ -420,7 +420,7 @@ func (s *ECRFetcherTestSuite) TestCreateFetcherErrorCases() { ctx := context.Background() err = ecrFetcher.Fetch(ctx, fetching.CycleMetadata{}) - s.Nil(err) + s.NoError(err) results := testhelper.CollectResources(s.resourceCh) s.Equal(0, len(results)) } From 5d2341c96116b65a43f441adacb247b4c1d572e6 Mon Sep 17 00:00:00 2001 From: Ofir Lapid Date: Sun, 24 Jul 2022 20:02:59 +0300 Subject: [PATCH 07/10] Code review changes --- resources/fetchers/ecr_fetcher.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/resources/fetchers/ecr_fetcher.go b/resources/fetchers/ecr_fetcher.go index 7bc67ba137..45c53956eb 100644 --- a/resources/fetchers/ecr_fetcher.go +++ b/resources/fetchers/ecr_fetcher.go @@ -35,7 +35,11 @@ import ( ) const ( + // PrivateRepoRegexTemplate should identify images with an ecr regex template + // .dkr.ecr..amazonaws.com/ PrivateRepoRegexTemplate = "^%s\\.dkr\\.ecr\\.([-\\w]+)\\.amazonaws\\.com\\/([-\\w\\.\\/]+)[:,@]?" + // PublicRepoRegex should identify images with a public ecr regex template + // public.ecr.aws// PublicRepoRegex = "public\\.ecr\\.aws\\/\\w+\\/([-\\w\\.\\/]+)\\:?" EcrRegionRegexGroup = 1 PublicEcrImageRegexIndex = 1 @@ -105,8 +109,9 @@ func (f *ECRFetcher) describePodImagesRepositories(ctx context.Context, podsList describedRepo, err := describer.Provider.DescribeRepositories(ctx, f.awsConfig, repositories, region) if err != nil { f.log.Errorf("could not retrieve pod's aws repositories for region %s: %w", region, err) + } else { + awsRepositories = append(awsRepositories, describedRepo.Repositories...) } - awsRepositories = append(awsRepositories, describedRepo.Repositories...) } return awsRepositories, nil } @@ -140,7 +145,8 @@ func ExtractRegionFromEcrImage(describer PodDescriber, image string) string { return "" } -// ExtractRegionFromPublicEcrImage TODO Ofir - https://github.com/elastic/security-team/issues/4035 +// ExtractRegionFromPublicEcrImage - Currently the public ecr provider is not functional. +// TODO - This method should be change as part of implementing the public ecr - https://github.com/elastic/security-team/issues/4035 func ExtractRegionFromPublicEcrImage(_ PodDescriber, _ string) string { return "" } From 455e6fbdc5aeecaf2e501504d2fc4896f516c7c3 Mon Sep 17 00:00:00 2001 From: Ofir Lapid Date: Sun, 24 Jul 2022 20:05:01 +0300 Subject: [PATCH 08/10] Adding comments to tests --- resources/fetchers/ecr_factory_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/resources/fetchers/ecr_factory_test.go b/resources/fetchers/ecr_factory_test.go index d94ae96a3b..df06f03630 100644 --- a/resources/fetchers/ecr_factory_test.go +++ b/resources/fetchers/ecr_factory_test.go @@ -68,7 +68,12 @@ default_region: us1-east "us1-east", "my-account", []string{ + + // this regex should identify images with an ecr regex template + // .dkr.ecr..amazonaws.com/ "^my-account\\.dkr\\.ecr\\.([-\\w]+)\\.amazonaws\\.com\\/([-\\w\\.\\/]+)[:,@]?", + // this regex should identify images with a public ecr regex template + // public.ecr.aws// "public\\.ecr\\.aws\\/\\w+\\/([-\\w\\.\\/]+)\\:?", }, }, From a7cc6ee7427eb5c3beafb49bcf31f3b75b25e589 Mon Sep 17 00:00:00 2001 From: Ofir Lapid Date: Mon, 25 Jul 2022 15:13:18 +0300 Subject: [PATCH 09/10] Stabilizing the flaky test --- resources/fetchers/ecr_fetcher_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/resources/fetchers/ecr_fetcher_test.go b/resources/fetchers/ecr_fetcher_test.go index fb1f9f7ed9..a135fca71f 100644 --- a/resources/fetchers/ecr_fetcher_test.go +++ b/resources/fetchers/ecr_fetcher_test.go @@ -23,6 +23,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/elastic/cloudbeat/resources/fetching" "regexp" + "sort" "testing" "github.com/aws/aws-sdk-go-v2/service/ecr" @@ -185,7 +186,7 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { }}, }, }, - []string{firstRepositoryName, publicRepoName}, + []string{publicRepoName, firstRepositoryName}, }, { "123456789123", @@ -331,6 +332,13 @@ func (s *ECRFetcherTestSuite) TestCreateFetcher() { s.Equal(len(test.expectedRepositoriesNames), len(results)) s.NoError(err) + // DO NOT REMOVE THIS METHOD + // The results provided by the channel can return the results in a different order + // This method order the results before asserting on them + sort.SliceStable(results, func(i, j int) bool { + return *results[i].Resource.(ECRResource).RepositoryName <= *results[j].Resource.(ECRResource).RepositoryName + }) + for i, name := range test.expectedRepositoriesNames { ecrResource := results[i].Resource.(ECRResource) metadata, err := ecrResource.GetMetadata() From 388d4622d49d16b202515b8006fe59e4af116476 Mon Sep 17 00:00:00 2001 From: Ofir Lapid Date: Mon, 25 Jul 2022 18:11:14 +0300 Subject: [PATCH 10/10] adding error to Extract region interface --- resources/fetchers/ecr_fetcher.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/resources/fetchers/ecr_fetcher.go b/resources/fetchers/ecr_fetcher.go index 45c53956eb..96e7743035 100644 --- a/resources/fetchers/ecr_fetcher.go +++ b/resources/fetchers/ecr_fetcher.go @@ -58,7 +58,7 @@ type ECRFetcher struct { type PodDescriber struct { FilterRegex *regexp.Regexp Provider awslib.EcrRepositoryDescriber - ExtractRegion func(describer PodDescriber, repo string) string + ExtractRegion func(describer PodDescriber, repo string) (string, error) ImageRegexIndex int } @@ -127,7 +127,11 @@ func getAwsRepositories(podsList *v1.PodList, describer PodDescriber) map[string regexMatcher := describer.FilterRegex.FindStringSubmatch(image) if regexMatcher != nil { repository := regexMatcher[describer.ImageRegexIndex] - region := describer.ExtractRegion(describer, image) + region, err := describer.ExtractRegion(describer, image) + if err != nil { + logp.Error(err) + continue + } reposByRegion[region] = append(reposByRegion[region], repository) } } @@ -135,20 +139,20 @@ func getAwsRepositories(podsList *v1.PodList, describer PodDescriber) map[string return reposByRegion } -func ExtractRegionFromEcrImage(describer PodDescriber, image string) string { +func ExtractRegionFromEcrImage(describer PodDescriber, image string) (string, error) { regexMatcher := describer.FilterRegex.FindStringSubmatch(image) if regexMatcher != nil { repository := regexMatcher[EcrRegionRegexGroup] - return repository + return repository, nil } - return "" + return "", fmt.Errorf("could not extract region, image does not match the aws ecr template") } // ExtractRegionFromPublicEcrImage - Currently the public ecr provider is not functional. // TODO - This method should be change as part of implementing the public ecr - https://github.com/elastic/security-team/issues/4035 -func ExtractRegionFromPublicEcrImage(_ PodDescriber, _ string) string { - return "" +func ExtractRegionFromPublicEcrImage(_ PodDescriber, _ string) (string, error) { + return "", nil } func (res ECRResource) GetData() interface{} {