From da620925656b1d6518eb46834cc2174f1f51921d Mon Sep 17 00:00:00 2001 From: Raphael Ludwig Date: Tue, 5 Jul 2022 13:05:03 +0200 Subject: [PATCH 1/9] feat: Utilize gitCommitID from cloud events to fetch resources Signed-off-by: Raphael Ludwig --- .../main.go | 14 ++- cmd/job-executor-service/main.go | 10 +- pkg/config/fake/reader_mock.go | 12 +-- pkg/config/reader.go | 10 +- pkg/config/reader_test.go | 12 +-- pkg/eventhandler/eventhandlers.go | 17 ++- pkg/eventhandler/eventhandlers_test.go | 14 +-- pkg/eventhandler/eventmapper.go | 3 + pkg/eventhandler/fake/eventhandlers_mock.go | 8 +- pkg/file/file_test.go | 2 +- pkg/k8sutils/job.go | 13 ++- pkg/k8sutils/job_test.go | 12 ++- pkg/keptn/config_service.go | 79 ++++++++++---- pkg/keptn/config_service_test.go | 99 ++++++++++++----- pkg/keptn/fake/config_service_mock.go | 44 ++++---- pkg/keptn/resource_service.go | 59 +++++++++++ pkg/keptn/resource_service_test.go | 63 +++++++++++ test/data/e2e/gitcommitid.config.yaml | 12 +++ test/e2e/gitcommitid_test.go | 100 ++++++++++++++++++ 19 files changed, 464 insertions(+), 119 deletions(-) create mode 100644 pkg/keptn/resource_service.go create mode 100644 pkg/keptn/resource_service_test.go create mode 100644 test/data/e2e/gitcommitid.config.yaml create mode 100644 test/e2e/gitcommitid_test.go diff --git a/cmd/job-executor-service-initcontainer/main.go b/cmd/job-executor-service-initcontainer/main.go index bd43dcda..e566d3d6 100644 --- a/cmd/job-executor-service-initcontainer/main.go +++ b/cmd/job-executor-service-initcontainer/main.go @@ -13,7 +13,7 @@ import ( "time" "github.com/kelseyhightower/envconfig" - api "github.com/keptn/go-utils/pkg/api/utils" + api "github.com/keptn/go-utils/pkg/api/utils/v2" "github.com/spf13/afero" ) @@ -49,6 +49,8 @@ type envConfig struct { OAuthScopes []string `envconfig:"OAUTH_SCOPES" required:"false"` // The well known oauth discovery url for the init container OAuthDiscovery string `envconfig:"OAUTH_DISCOVERY" required:"false"` + // The gitCommitId of the initial cloud event, for older Keptn instances this might be empty + GitCommitID string `envconfig:"GIT_COMMIT_ID"` } func main() { @@ -118,7 +120,15 @@ func main() { useLocalFileSystem = true } - configService := keptn.NewConfigService(useLocalFileSystem, env.Project, env.Stage, env.Service, keptnAPI.ResourcesV1()) + // re-create the event from job-executor-service + eventProps := keptn.EventProperties{ + Project: env.Project, + Stage: env.Stage, + Service: env.Service, + GitCommitId: env.GitCommitID, + } + + configService := keptn.NewConfigService(useLocalFileSystem, eventProps, keptnAPI.Resources()) err = file.MountFiles(env.Action, env.Task, fs, configService) if err != nil { diff --git a/cmd/job-executor-service/main.go b/cmd/job-executor-service/main.go index 3ad6ae46..5f1f64f2 100644 --- a/cmd/job-executor-service/main.go +++ b/cmd/job-executor-service/main.go @@ -111,10 +111,12 @@ func processKeptnCloudEvent(ctx context.Context, event cloudevents.Event, allowL // create a uniform handler talking to the distributor uniformHandler := api.NewUniformHandler("localhost:8081/controlPlane") var eventHandler = &eventhandler.EventHandler{ - Keptn: myKeptn, - JobConfigReader: &config.JobConfigReader{Keptn: myKeptn}, - ServiceName: ServiceName, - Mapper: new(eventhandler.KeptnCloudEventMapper), + Keptn: myKeptn, + JobConfigReader: &config.JobConfigReader{ + Keptn: keptn_interface.NewV1ResourceHandler(myKeptn.Event, myKeptn.ResourceHandler), + }, + ServiceName: ServiceName, + Mapper: new(eventhandler.KeptnCloudEventMapper), ImageFilter: imageFilterImpl{ imageFilterList: allowList, }, diff --git a/pkg/config/fake/reader_mock.go b/pkg/config/fake/reader_mock.go index 80d65ea2..51d0509c 100644 --- a/pkg/config/fake/reader_mock.go +++ b/pkg/config/fake/reader_mock.go @@ -33,17 +33,17 @@ func (m *MockKeptnResourceService) EXPECT() *MockKeptnResourceServiceMockRecorde return m.recorder } -// GetKeptnResource mocks base method. -func (m *MockKeptnResourceService) GetKeptnResource(arg0 string) ([]byte, error) { +// GetResource mocks base method. +func (m *MockKeptnResourceService) GetResource(arg0, arg1 string) ([]byte, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetKeptnResource", arg0) + ret := m.ctrl.Call(m, "GetResource", arg0, arg1) ret0, _ := ret[0].([]byte) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetKeptnResource indicates an expected call of GetKeptnResource. -func (mr *MockKeptnResourceServiceMockRecorder) GetKeptnResource(arg0 interface{}) *gomock.Call { +// GetResource indicates an expected call of GetResource. +func (mr *MockKeptnResourceServiceMockRecorder) GetResource(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetKeptnResource", reflect.TypeOf((*MockKeptnResourceService)(nil).GetKeptnResource), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetResource", reflect.TypeOf((*MockKeptnResourceService)(nil).GetResource), arg0, arg1) } diff --git a/pkg/config/reader.go b/pkg/config/reader.go index 978ccbdf..813571d4 100644 --- a/pkg/config/reader.go +++ b/pkg/config/reader.go @@ -13,7 +13,7 @@ const jobConfigResourceName = "job/config.yaml" // KeptnResourceService defines the contract used by JobConfigReader to retrieve a resource from keptn (using project, // service, stage from context) type KeptnResourceService interface { - GetKeptnResource(resource string) ([]byte, error) + GetResource(resource string, gitCommitId string) ([]byte, error) } // JobConfigReader retrieves and parses job configuration from Keptn @@ -25,8 +25,9 @@ type JobConfigReader struct { // Additionally, also the SHA1 hash of the retrieved configuration will be returned. // In case of error retrieving the resource or parsing the yaml it will return (nil, // error) with the original error correctly wrapped in the local one -func (jcr *JobConfigReader) GetJobConfig() (*Config, string, error) { - resource, err := jcr.Keptn.GetKeptnResource(jobConfigResourceName) +func (jcr *JobConfigReader) GetJobConfig(gitCommitId string) (*Config, string, error) { + + resource, err := jcr.Keptn.GetResource(jobConfigResourceName, gitCommitId) if err != nil { return nil, "", fmt.Errorf("error retrieving job config: %w", err) } @@ -42,5 +43,8 @@ func (jcr *JobConfigReader) GetJobConfig() (*Config, string, error) { log.Printf("The config was: %s", string(resource)) return nil, "", fmt.Errorf("error parsing job configuration: %w", err) } + + log.Printf("config: SHA3: %s\nGIT :%s\n%s\n", resourceHash, gitCommitId, resource) + return configuration, resourceHash, nil } diff --git a/pkg/config/reader_test.go b/pkg/config/reader_test.go index b3d10bee..8484b69b 100644 --- a/pkg/config/reader_test.go +++ b/pkg/config/reader_test.go @@ -16,11 +16,11 @@ func TestConfigRetrievalFailed(t *testing.T) { mockKeptnResourceService := fake.NewMockKeptnResourceService(mockCtrl) retrievalError := errors.New("error getting resource") - mockKeptnResourceService.EXPECT().GetKeptnResource("job/config.yaml").Return(nil, retrievalError) + mockKeptnResourceService.EXPECT().GetResource("job/config.yaml", "c25692cb4fe4068fbdc2").Return(nil, retrievalError) sut := JobConfigReader{Keptn: mockKeptnResourceService} - config, _, err := sut.GetJobConfig() + config, _, err := sut.GetJobConfig("c25692cb4fe4068fbdc2") assert.ErrorIs(t, err, retrievalError) assert.Nil(t, config) } @@ -35,14 +35,14 @@ func TestMalformedConfig(t *testing.T) { has_nothing_to_do: with_job_executor: true ` - mockKeptnResourceService.EXPECT().GetKeptnResource("job/config.yaml").Return( + mockKeptnResourceService.EXPECT().GetResource("job/config.yaml", "").Return( []byte(yamlConfig), nil, ) sut := JobConfigReader{Keptn: mockKeptnResourceService} - config, _, err := sut.GetJobConfig() + config, _, err := sut.GetJobConfig("") assert.Error(t, err) assert.Nil(t, config) } @@ -64,14 +64,14 @@ func TestGetConfigHappyPath(t *testing.T) { cmd: - echo "Hello World!" ` - mockKeptnResourceService.EXPECT().GetKeptnResource("job/config.yaml").Return( + mockKeptnResourceService.EXPECT().GetResource("job/config.yaml", "c25692cb4fe4068fbdc2").Return( []byte(yamlConfig), nil, ) sut := JobConfigReader{Keptn: mockKeptnResourceService} - config, _, err := sut.GetJobConfig() + config, _, err := sut.GetJobConfig("c25692cb4fe4068fbdc2") assert.NoError(t, err) assert.NotNil(t, config) } diff --git a/pkg/eventhandler/eventhandlers.go b/pkg/eventhandler/eventhandlers.go index 301a9fa6..30e4818c 100644 --- a/pkg/eventhandler/eventhandlers.go +++ b/pkg/eventhandler/eventhandlers.go @@ -38,7 +38,7 @@ type EventMapper interface { // JobConfigReader retrieves the job-executor-service configuration type JobConfigReader interface { - GetJobConfig() (*config.Config, string, error) + GetJobConfig(gitCommitId string) (*config.Config, string, error) } // ErrorLogSender is used to send error logs that will appear in Uniform UI @@ -97,7 +97,13 @@ func (eh *EventHandler) HandleEvent() error { ) log.Printf("CloudEvent %T: %v", eventAsInterface, eventAsInterface) - configuration, configHash, err := eh.JobConfigReader.GetJobConfig() + // Get the git commit id from the cloud event (if it exists) and use it to query the job configuration + var gitCommitId string + if commitId, ok := eventAsInterface["gitcommitid"]; ok { + gitCommitId, _ = commitId.(string) + } + + configuration, configHash, err := eh.JobConfigReader.GetJobConfig(gitCommitId) if err != nil { errorLogErr := eh.ErrorSender.SendErrorLogEvent( @@ -134,14 +140,16 @@ func (eh *EventHandler) HandleEvent() error { eh.Keptn.CloudEvent.Type(), action.Name, ) - eh.startK8sJob(&action, actionIndex, configHash, eventAsInterface) + eh.startK8sJob(&action, actionIndex, configHash, gitCommitId, eventAsInterface) } } return nil } -func (eh *EventHandler) startK8sJob(action *config.Action, actionIndex int, configHash string, jsonEventData interface{}) { +func (eh *EventHandler) startK8sJob(action *config.Action, actionIndex int, configHash string, gitCommitId string, + jsonEventData interface{}, +) { if !action.Silent { _, err := eh.Keptn.SendTaskStartedEvent(nil, eh.ServiceName) @@ -199,6 +207,7 @@ func (eh *EventHandler) startK8sJob(action *config.Action, actionIndex int, conf ActionIndex: actionIndex, TaskIndex: index, JobConfigHash: configHash, + GitCommitId: gitCommitId, } err = eh.K8s.CreateK8sJob( diff --git a/pkg/eventhandler/eventhandlers_test.go b/pkg/eventhandler/eventhandlers_test.go index a37658ba..4ac9c5e8 100644 --- a/pkg/eventhandler/eventhandlers_test.go +++ b/pkg/eventhandler/eventhandlers_test.go @@ -151,7 +151,7 @@ func TestErrorGettingJobConfig(t *testing.T) { mockUniformErrorSender := eventhandlerfake.NewMockErrorLogSender(mockCtrl) errorGettingJobConfig := errors.New("error getting resource") - mockJobConfigReader.EXPECT().GetJobConfig().Return( + mockJobConfigReader.EXPECT().GetJobConfig("").Return( nil, "", errorGettingJobConfig, ).Times(1) @@ -211,7 +211,7 @@ func TestErrorConnectingToK8s(t *testing.T) { mockJobConfigReader := eventhandlerfake.NewMockJobConfigReader(mockCtrl) mockUniformErrorSender := eventhandlerfake.NewMockErrorLogSender(mockCtrl) - mockJobConfigReader.EXPECT().GetJobConfig().Return( + mockJobConfigReader.EXPECT().GetJobConfig("").Return( &config.Config{ APIVersion: &apiVersion, Actions: []config.Action{ @@ -375,7 +375,7 @@ func TestEventMatching(t *testing.T) { } mockJobConfigReader := eventhandlerfake.NewMockJobConfigReader(mockCtrl) - mockJobConfigReader.EXPECT().GetJobConfig().Return( + mockJobConfigReader.EXPECT().GetJobConfig("").Return( &config, "", nil, ).Times(1) @@ -510,7 +510,7 @@ func TestStartK8s(t *testing.T) { k8sMock.EXPECT().GetLogsOfPod(gomock.Eq(jobName1), jobNamespace1).Times(1) k8sMock.EXPECT().GetLogsOfPod(gomock.Eq(jobName2), jobNamespace2).Times(1) - eh.startK8sJob(&action, 0, "", eventPayloadAsInterface) + eh.startK8sJob(&action, 0, "", "", eventPayloadAsInterface) err = fakeEventSender.AssertSentEventTypes( []string{ @@ -561,7 +561,7 @@ func TestStartK8sJobSilent(t *testing.T) { k8sMock.EXPECT().GetLogsOfPod(gomock.Eq(jobName1), gomock.Any()).Times(1) k8sMock.EXPECT().GetLogsOfPod(gomock.Eq(jobName2), gomock.Any()).Times(1) - eh.startK8sJob(&action, 0, "", eventPayloadAsInterface) + eh.startK8sJob(&action, 0, "", "", eventPayloadAsInterface) err = fakeEventSender.AssertSentEventTypes([]string{}) assert.NoError(t, err) @@ -605,7 +605,7 @@ func TestStartK8s_TestFinishedEvent(t *testing.T) { require.NoError(t, err) time.Local = local - eh.startK8sJob(&action, 0, "", eventPayloadAsInterface) + eh.startK8sJob(&action, 0, "", "", eventPayloadAsInterface) err = fakeEventSender.AssertSentEventTypes( []string{ @@ -677,7 +677,7 @@ func TestExpectImageNotAllowedError(t *testing.T) { require.NoError(t, err) time.Local = local - eh.startK8sJob(&action, 0, "", eventPayloadAsInterface) + eh.startK8sJob(&action, 0, "", "", eventPayloadAsInterface) err = fakeEventSender.AssertSentEventTypes( []string{ diff --git a/pkg/eventhandler/eventmapper.go b/pkg/eventhandler/eventmapper.go index 777f201a..a13fc7cf 100644 --- a/pkg/eventhandler/eventmapper.go +++ b/pkg/eventhandler/eventmapper.go @@ -26,6 +26,8 @@ func (kcem *KeptnCloudEventMapper) Map(ce cloudevents.Event) (map[string]interfa extension, _ := ce.Context.GetExtension("shkeptncontext") shKeptnContext := extension.(string) + gitcommitidExt, _ := ce.Context.GetExtension("gitcommitid") + eventAsInterface := make(map[string]interface{}) eventAsInterface["id"] = ce.ID() eventAsInterface["shkeptncontext"] = shKeptnContext @@ -34,6 +36,7 @@ func (kcem *KeptnCloudEventMapper) Map(ce cloudevents.Event) (map[string]interfa eventAsInterface["data"] = eventDataAsInterface eventAsInterface["specversion"] = ce.SpecVersion() eventAsInterface["type"] = ce.Type() + eventAsInterface["gitcommitid"] = gitcommitidExt return eventAsInterface, nil } diff --git a/pkg/eventhandler/fake/eventhandlers_mock.go b/pkg/eventhandler/fake/eventhandlers_mock.go index 38d6bd52..b3c7e1b8 100644 --- a/pkg/eventhandler/fake/eventhandlers_mock.go +++ b/pkg/eventhandler/fake/eventhandlers_mock.go @@ -114,9 +114,9 @@ func (m *MockJobConfigReader) EXPECT() *MockJobConfigReaderMockRecorder { } // GetJobConfig mocks base method. -func (m *MockJobConfigReader) GetJobConfig() (*config.Config, string, error) { +func (m *MockJobConfigReader) GetJobConfig(arg0 string) (*config.Config, string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetJobConfig") + ret := m.ctrl.Call(m, "GetJobConfig", arg0) ret0, _ := ret[0].(*config.Config) ret1, _ := ret[1].(string) ret2, _ := ret[2].(error) @@ -124,9 +124,9 @@ func (m *MockJobConfigReader) GetJobConfig() (*config.Config, string, error) { } // GetJobConfig indicates an expected call of GetJobConfig. -func (mr *MockJobConfigReaderMockRecorder) GetJobConfig() *gomock.Call { +func (mr *MockJobConfigReaderMockRecorder) GetJobConfig(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetJobConfig", reflect.TypeOf((*MockJobConfigReader)(nil).GetJobConfig)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetJobConfig", reflect.TypeOf((*MockJobConfigReader)(nil).GetJobConfig), arg0) } // MockK8s is a mock of K8s interface. diff --git a/pkg/file/file_test.go b/pkg/file/file_test.go index 658d8123..f56acb8f 100644 --- a/pkg/file/file_test.go +++ b/pkg/file/file_test.go @@ -155,7 +155,7 @@ func TestMountFilesFileNotFound(t *testing.T) { func TestMountFilesWithLocalFileSystem(t *testing.T) { fs := afero.NewMemMapFs() - configService := keptn.NewConfigService(true, "", "", "", nil) + configService := keptn.NewConfigService(true, keptn.EventProperties{}, nil) err := afero.WriteFile(fs, "job/config.yaml", []byte(simpleConfig), 0644) assert.NoError(t, err) err = afero.WriteFile(fs, "/helm/values.yaml", []byte("here be awesome configuration"), 0644) diff --git a/pkg/k8sutils/job.go b/pkg/k8sutils/job.go index 58750d58..4554bf0a 100644 --- a/pkg/k8sutils/job.go +++ b/pkg/k8sutils/job.go @@ -60,6 +60,7 @@ type JobDetails struct { ActionIndex int TaskIndex int JobConfigHash string + GitCommitId string } // JobSettings contains environment variable settings for the job @@ -323,6 +324,10 @@ func (k8s *K8sImpl) CreateK8sJob( Name: "JOB_TASK", Value: task.Name, }, + { + Name: "GIT_COMMIT_ID", + Value: jobDetails.GitCommitId, + }, }, Resources: *jobSettings.DefaultResourceRequirements, }, @@ -614,12 +619,6 @@ func generateK8sJobLabels(jobDetails JobDetails, jsonEventData interface{}, jesD return nil, fmt.Errorf("jsonEventData does not contain the field id") } - gitCommitID, ok := eventAsMap["gitcommitid"].(string) - if !ok { - // For legacy events that have no git commit id we just set it to an empty string - gitCommitID = "" - } - // This function is used to sanitize the labels for the action and the task name to // avoid creating a set of labels that is not allowed by kubernetes sanitizeLabel := func(label string) string { @@ -642,7 +641,7 @@ func generateK8sJobLabels(jobDetails JobDetails, jsonEventData interface{}, jesD "app.kubernetes.io/managed-by": jesDeploymentName, "keptn.sh/context": keptnContext, "keptn.sh/event-id": eventID, - "keptn.sh/commitid": gitCommitID, + "keptn.sh/commitid": jobDetails.GitCommitId, "keptn.sh/jes-action": sanitizeLabel(jobDetails.Action.Name), "keptn.sh/jes-task": sanitizeLabel(jobDetails.Task.Name), "keptn.sh/jes-job-confighash": jobDetails.JobConfigHash, diff --git a/pkg/k8sutils/job_test.go b/pkg/k8sutils/job_test.go index b372e962..058f1ac0 100644 --- a/pkg/k8sutils/job_test.go +++ b/pkg/k8sutils/job_test.go @@ -1127,6 +1127,11 @@ func TestCreateK8sJobContainsCorrectLabels(t *testing.T) { t.Run(test.name, func(t *testing.T) { jobName := "some-job-name-" + strconv.Itoa(i) + var gitCommitId string + if id, ok := test.event["gitCommitid"]; ok { + gitCommitId = id.(string) + } + err = k8s.CreateK8sJob( jobName, JobDetails{ @@ -1141,6 +1146,7 @@ func TestCreateK8sJobContainsCorrectLabels(t *testing.T) { ActionIndex: 0, TaskIndex: 0, JobConfigHash: "", + GitCommitId: gitCommitId, }, &eventData, JobSettings{ @@ -1165,7 +1171,7 @@ func TestCreateK8sJobContainsCorrectLabels(t *testing.T) { "app.kubernetes.io/managed-by": "job-executor-service", "keptn.sh/context": test.event["shkeptncontext"].(string), "keptn.sh/event-id": test.event["id"].(string), - "keptn.sh/commitid": "", + "keptn.sh/commitid": gitCommitId, "keptn.sh/jes-action": test.expectedActionName, "keptn.sh/jes-task": test.expectedTaskName, "keptn.sh/jes-job-confighash": "", @@ -1173,10 +1179,6 @@ func TestCreateK8sJobContainsCorrectLabels(t *testing.T) { "keptn.sh/jes-task-index": "0", } - if test.event["gitcommitid"] != nil { - expectedLabels["keptn.sh/commitid"] = test.event["gitcommitid"].(string) - } - assert.Equal(t, expectedLabels, job.Labels) }) } diff --git a/pkg/keptn/config_service.go b/pkg/keptn/config_service.go index c9630910..768b22d8 100644 --- a/pkg/keptn/config_service.go +++ b/pkg/keptn/config_service.go @@ -1,12 +1,14 @@ package keptn import ( + "context" "fmt" + "github.com/keptn/go-utils/pkg/api/models" + api "github.com/keptn/go-utils/pkg/api/utils/v2" "net/url" "os" "strings" - "github.com/keptn/go-utils/pkg/api/models" "github.com/spf13/afero" ) @@ -18,31 +20,36 @@ type ConfigService interface { GetAllKeptnResources(fs afero.Fs, resource string) (map[string][]byte, error) } -//go:generate mockgen -source=config_service.go -destination=fake/config_service_mock.go -package=fake ResourceHandler +//go:generate mockgen -source=config_service.go -destination=fake/config_service_mock.go -package=fake V2ResourceHandler + +// V2ResourceHandler provides methods to work with the keptn configuration service +type V2ResourceHandler interface { + // GetAllServiceResources returns a list of all resources. + GetAllServiceResources(ctx context.Context, project string, stage string, service string, + opts api.ResourcesGetAllServiceResourcesOptions) ([]*models.Resource, error) -// ResourceHandler provides methods to work with the keptn configuration service -type ResourceHandler interface { - GetServiceResource(project string, stage string, service string, resourceURI string) (*models.Resource, error) - GetAllServiceResources(project string, stage string, service string) ([]*models.Resource, error) + // GetResource returns a resource from the defined ResourceScope. + GetResource(ctx context.Context, scope api.ResourceScope, opts api.ResourcesGetResourceOptions) (*models.Resource, error) } type configServiceImpl struct { useLocalFileSystem bool - project string - stage string - service string - resourceHandler ResourceHandler + eventProperties EventProperties + resourceHandler V2ResourceHandler +} + +type EventProperties struct { + Project string + Stage string + Service string + GitCommitId string } // NewConfigService creates and returns new ConfigService -func NewConfigService( - useLocalFileSystem bool, project string, stage string, service string, resourceHandler ResourceHandler, -) ConfigService { +func NewConfigService(useLocalFileSystem bool, event EventProperties, resourceHandler V2ResourceHandler) ConfigService { return &configServiceImpl{ useLocalFileSystem: useLocalFileSystem, - project: project, - stage: stage, - service: service, + eventProperties: event, resourceHandler: resourceHandler, } } @@ -55,11 +62,25 @@ func (k *configServiceImpl) GetKeptnResource(fs afero.Fs, resource string) ([]by return k.getKeptnResourceFromLocal(fs, resource) } - // get it from KeptnBase // https://github.com/keptn/keptn/issues/2707 - requestedResource, err := k.resourceHandler.GetServiceResource( - k.project, k.stage, k.service, url.QueryEscape(resource), - ) + encodedResource := url.QueryEscape(resource) + + scope := api.NewResourceScope() + scope.Project(k.eventProperties.Project) + scope.Stage(k.eventProperties.Stage) + scope.Service(k.eventProperties.Service) + scope.Resource(encodedResource) + + options := api.ResourcesGetResourceOptions{} + if k.eventProperties.GitCommitId != "" { + options.URIOptions = []api.URIOption{ + api.AppendQuery(url.Values{ + "gitCommitID": []string{k.eventProperties.GitCommitId}, + }), + } + } + + requestedResource, err := k.resourceHandler.GetResource(context.Background(), *scope, options) // return Nil in case resource couldn't be retrieved if err != nil || requestedResource.ResourceContent == "" { @@ -78,22 +99,34 @@ func (k *configServiceImpl) GetAllKeptnResources(fs afero.Fs, resource string) ( return k.getKeptnResourcesFromLocal(fs, resource) } - // get it from KeptnBase - requestedResources, err := k.resourceHandler.GetAllServiceResources(k.project, k.stage, k.service) + scope := api.NewResourceScope() + scope.Project(k.eventProperties.Project) + scope.Stage(k.eventProperties.Stage) + scope.Service(k.eventProperties.Service) + + // Get all resources from Keptn in the current service + requestedResources, err := k.resourceHandler.GetAllServiceResources(context.Background(), + k.eventProperties.Project, k.eventProperties.Stage, k.eventProperties.Service, + api.ResourcesGetAllServiceResourcesOptions{}, + ) if err != nil { return nil, fmt.Errorf("resources not found: %s", err) } + // Go over the resources and fetch the content of each resource with the given gitCommitId: keptnResources := make(map[string][]byte) for _, serviceResource := range requestedResources { // match against with and without starting slash + // Note: this makes it possible to include directories, maybe a glob might be a better idea resourceURIWithoutSlash := strings.Replace(*serviceResource.ResourceURI, "/", "", 1) if strings.HasPrefix(*serviceResource.ResourceURI, resource) || strings.HasPrefix( resourceURIWithoutSlash, resource, ) { keptnResourceContent, err := k.GetKeptnResource(fs, *serviceResource.ResourceURI) if err != nil { - return nil, fmt.Errorf("could not find file %s", *serviceResource.ResourceURI) + return nil, fmt.Errorf("could not find file %s for version %s", + *serviceResource.ResourceURI, k.eventProperties.GitCommitId, + ) } keptnResources[*serviceResource.ResourceURI] = keptnResourceContent } diff --git a/pkg/keptn/config_service_test.go b/pkg/keptn/config_service_test.go index 7438bb2a..f9ff681e 100644 --- a/pkg/keptn/config_service_test.go +++ b/pkg/keptn/config_service_test.go @@ -1,6 +1,7 @@ package keptn import ( + api "github.com/keptn/go-utils/pkg/api/utils/v2" "net/url" "path" "testing" @@ -15,23 +16,24 @@ import ( "github.com/spf13/afero" ) -func CreateResourceHandlerMock(t *testing.T) *keptnfake.MockResourceHandler { +func CreateResourceHandlerMock(t *testing.T) *keptnfake.MockV2ResourceHandler { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - return keptnfake.NewMockResourceHandler(mockCtrl) + return keptnfake.NewMockV2ResourceHandler(mockCtrl) } const service = "carts" const project = "sockshop" const stage = "dev" +const gitCommitId = "6caf78d2c978f7f787" func TestGetAllKeptnResources(t *testing.T) { locustBasic := "/locust/basic.py" locustFunctional := "/locust/functional.py" resourceHandlerMock := CreateResourceHandlerMock(t) - resourceHandlerMock.EXPECT().GetAllServiceResources(project, stage, service).Times(1).Return( + resourceHandlerMock.EXPECT().GetAllServiceResources(gomock.Any(), project, stage, service, gomock.Any()).Times(1).Return( []*models.Resource{ { Metadata: nil, @@ -46,9 +48,13 @@ func TestGetAllKeptnResources(t *testing.T) { }, nil, ) - resourceHandlerMock.EXPECT().GetServiceResource( - project, stage, service, url.QueryEscape(locustBasic), - ).Times(1).Return( + scope1 := api.NewResourceScope() + scope1.Project(project) + scope1.Stage(stage) + scope1.Service(service) + scope1.Resource(url.QueryEscape(locustBasic)) + + resourceHandlerMock.EXPECT().GetResource(gomock.Any(), *scope1, gomock.Any()).Times(1).Return( &models.Resource{ Metadata: nil, ResourceContent: locustBasic, @@ -56,9 +62,13 @@ func TestGetAllKeptnResources(t *testing.T) { }, nil, ) - resourceHandlerMock.EXPECT().GetServiceResource( - project, stage, service, url.QueryEscape(locustFunctional), - ).Times(1).Return( + scope2 := api.NewResourceScope() + scope2.Project(project) + scope2.Stage(stage) + scope2.Service(service) + scope2.Resource(url.QueryEscape(locustFunctional)) + + resourceHandlerMock.EXPECT().GetResource(gomock.Any(), *scope2, gomock.Any()).Times(1).Return( &models.Resource{ Metadata: nil, ResourceContent: locustFunctional, @@ -66,7 +76,14 @@ func TestGetAllKeptnResources(t *testing.T) { }, nil, ) - configService := NewConfigService(false, project, stage, service, resourceHandlerMock) + event := EventProperties{ + Project: project, + Stage: stage, + Service: service, + GitCommitId: gitCommitId, + } + + configService := NewConfigService(false, event, resourceHandlerMock) fs := afero.NewMemMapFs() keptnResources, err := configService.GetAllKeptnResources(fs, "locust") @@ -87,17 +104,32 @@ func TestGetAllKeptnResourcesLocal(t *testing.T) { locustFunctional := path.Join(locustPath, "functional.py") resourceHandlerMock := CreateResourceHandlerMock(t) - resourceHandlerMock.EXPECT().GetAllServiceResources(project, stage, service).Times(0) + resourceHandlerMock.EXPECT().GetAllServiceResources(gomock.Any(), project, stage, service, gomock.Any()).Times(0) + + scope1 := api.NewResourceScope() + scope1.Project(project) + scope1.Stage(stage) + scope1.Service(service) + scope1.Resource(url.QueryEscape(locustBasic)) + + resourceHandlerMock.EXPECT().GetResource(gomock.Any(), scope1, gomock.Any()).Times(0) - resourceHandlerMock.EXPECT().GetServiceResource( - project, stage, service, url.QueryEscape(locustBasic), - ).Times(0) + scope2 := api.NewResourceScope() + scope2.Project(project) + scope2.Stage(stage) + scope2.Service(service) + scope2.Resource(url.QueryEscape(locustFunctional)) - resourceHandlerMock.EXPECT().GetServiceResource( - project, stage, service, url.QueryEscape(locustFunctional), - ).Times(0) + resourceHandlerMock.EXPECT().GetResource(gomock.Any(), scope2, gomock.Any()).Times(0) - configService := NewConfigService(true, project, stage, service, resourceHandlerMock) + event := EventProperties{ + Project: project, + Stage: stage, + Service: service, + GitCommitId: "", + } + + configService := NewConfigService(true, event, resourceHandlerMock) fs := afero.NewMemMapFs() err := createFile(fs, locustBasic, []byte(locustBasic)) @@ -124,17 +156,32 @@ func TestErrorNoDirectoryResourcesLocal(t *testing.T) { locustFunctional := path.Join(locustPath, "functional.py") resourceHandlerMock := CreateResourceHandlerMock(t) - resourceHandlerMock.EXPECT().GetAllServiceResources(project, stage, service).Times(0) + resourceHandlerMock.EXPECT().GetAllServiceResources(gomock.Any(), project, stage, service, gomock.Any()).Times(0) + + scope1 := api.NewResourceScope() + scope1.Project(project) + scope1.Stage(stage) + scope1.Service(service) + scope1.Resource(url.QueryEscape(locustBasic)) - resourceHandlerMock.EXPECT().GetServiceResource( - project, stage, service, url.QueryEscape(locustBasic), - ).Times(0) + resourceHandlerMock.EXPECT().GetResource(gomock.Any(), scope1, gomock.Any()).Times(0) - resourceHandlerMock.EXPECT().GetServiceResource( - project, stage, service, url.QueryEscape(locustFunctional), - ).Times(0) + scope2 := api.NewResourceScope() + scope2.Project(project) + scope2.Stage(stage) + scope2.Service(service) + scope2.Resource(url.QueryEscape(locustFunctional)) + + resourceHandlerMock.EXPECT().GetResource(gomock.Any(), scope2, gomock.Any()).Times(0) + + event := EventProperties{ + Project: project, + Stage: stage, + Service: service, + GitCommitId: "", + } - configService := NewConfigService(true, project, stage, service, resourceHandlerMock) + configService := NewConfigService(true, event, resourceHandlerMock) fs := afero.NewMemMapFs() _, err := configService.GetAllKeptnResources(fs, locustPath) diff --git a/pkg/keptn/fake/config_service_mock.go b/pkg/keptn/fake/config_service_mock.go index 8fd07d35..cc1af966 100644 --- a/pkg/keptn/fake/config_service_mock.go +++ b/pkg/keptn/fake/config_service_mock.go @@ -5,10 +5,12 @@ package fake import ( + context "context" reflect "reflect" gomock "github.com/golang/mock/gomock" models "github.com/keptn/go-utils/pkg/api/models" + v2 "github.com/keptn/go-utils/pkg/api/utils/v2" afero "github.com/spf13/afero" ) @@ -65,55 +67,55 @@ func (mr *MockConfigServiceMockRecorder) GetKeptnResource(fs, resource interface return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetKeptnResource", reflect.TypeOf((*MockConfigService)(nil).GetKeptnResource), fs, resource) } -// MockResourceHandler is a mock of ResourceHandler interface. -type MockResourceHandler struct { +// MockV2ResourceHandler is a mock of V2ResourceHandler interface. +type MockV2ResourceHandler struct { ctrl *gomock.Controller - recorder *MockResourceHandlerMockRecorder + recorder *MockV2ResourceHandlerMockRecorder } -// MockResourceHandlerMockRecorder is the mock recorder for MockResourceHandler. -type MockResourceHandlerMockRecorder struct { - mock *MockResourceHandler +// MockV2ResourceHandlerMockRecorder is the mock recorder for MockV2ResourceHandler. +type MockV2ResourceHandlerMockRecorder struct { + mock *MockV2ResourceHandler } -// NewMockResourceHandler creates a new mock instance. -func NewMockResourceHandler(ctrl *gomock.Controller) *MockResourceHandler { - mock := &MockResourceHandler{ctrl: ctrl} - mock.recorder = &MockResourceHandlerMockRecorder{mock} +// NewMockV2ResourceHandler creates a new mock instance. +func NewMockV2ResourceHandler(ctrl *gomock.Controller) *MockV2ResourceHandler { + mock := &MockV2ResourceHandler{ctrl: ctrl} + mock.recorder = &MockV2ResourceHandlerMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockResourceHandler) EXPECT() *MockResourceHandlerMockRecorder { +func (m *MockV2ResourceHandler) EXPECT() *MockV2ResourceHandlerMockRecorder { return m.recorder } // GetAllServiceResources mocks base method. -func (m *MockResourceHandler) GetAllServiceResources(project, stage, service string) ([]*models.Resource, error) { +func (m *MockV2ResourceHandler) GetAllServiceResources(ctx context.Context, project, stage, service string, opts v2.ResourcesGetAllServiceResourcesOptions) ([]*models.Resource, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetAllServiceResources", project, stage, service) + ret := m.ctrl.Call(m, "GetAllServiceResources", ctx, project, stage, service, opts) ret0, _ := ret[0].([]*models.Resource) ret1, _ := ret[1].(error) return ret0, ret1 } // GetAllServiceResources indicates an expected call of GetAllServiceResources. -func (mr *MockResourceHandlerMockRecorder) GetAllServiceResources(project, stage, service interface{}) *gomock.Call { +func (mr *MockV2ResourceHandlerMockRecorder) GetAllServiceResources(ctx, project, stage, service, opts interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllServiceResources", reflect.TypeOf((*MockResourceHandler)(nil).GetAllServiceResources), project, stage, service) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllServiceResources", reflect.TypeOf((*MockV2ResourceHandler)(nil).GetAllServiceResources), ctx, project, stage, service, opts) } -// GetServiceResource mocks base method. -func (m *MockResourceHandler) GetServiceResource(project, stage, service, resourceURI string) (*models.Resource, error) { +// GetResource mocks base method. +func (m *MockV2ResourceHandler) GetResource(ctx context.Context, scope v2.ResourceScope, opts v2.ResourcesGetResourceOptions) (*models.Resource, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetServiceResource", project, stage, service, resourceURI) + ret := m.ctrl.Call(m, "GetResource", ctx, scope, opts) ret0, _ := ret[0].(*models.Resource) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetServiceResource indicates an expected call of GetServiceResource. -func (mr *MockResourceHandlerMockRecorder) GetServiceResource(project, stage, service, resourceURI interface{}) *gomock.Call { +// GetResource indicates an expected call of GetResource. +func (mr *MockV2ResourceHandlerMockRecorder) GetResource(ctx, scope, opts interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetServiceResource", reflect.TypeOf((*MockResourceHandler)(nil).GetServiceResource), project, stage, service, resourceURI) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetResource", reflect.TypeOf((*MockV2ResourceHandler)(nil).GetResource), ctx, scope, opts) } diff --git a/pkg/keptn/resource_service.go b/pkg/keptn/resource_service.go new file mode 100644 index 00000000..8a43f0e0 --- /dev/null +++ b/pkg/keptn/resource_service.go @@ -0,0 +1,59 @@ +package keptn + +import ( + "fmt" + "github.com/keptn/go-utils/pkg/api/models" + api "github.com/keptn/go-utils/pkg/api/utils" + "github.com/keptn/go-utils/pkg/lib/keptn" + "net/url" +) + +// V1ResourceHandler is a wrapper around the v1 ResourceHandler of the Keptn API to simplify the +// getting of resources of a given event +type V1ResourceHandler struct { + Event EventProperties + ResourceHandler KeptnResourceHandler +} + +func NewV1ResourceHandler(event keptn.EventProperties, handler KeptnResourceHandler) V1ResourceHandler { + return V1ResourceHandler{ + Event: EventProperties{ + Project: event.GetProject(), + Stage: event.GetStage(), + Service: event.GetService(), + }, + ResourceHandler: handler, + } +} + +//go:generate mockgen -source=resource_service.go -destination=fake/keptn_resourcehandler_mock.go -package=fake KeptnResourceHandler + +// KeptnResourceHandler represents an interface for the api.ResourceHandler struct of the Keptn API +type KeptnResourceHandler interface { + GetResource(scope api.ResourceScope, options ...api.URIOption) (*models.Resource, error) +} + +// GetResource returns the contents of a resource for a given gitCommitId +func (r V1ResourceHandler) GetResource(resource string, gitCommitId string) ([]byte, error) { + scope := api.NewResourceScope() + scope.Resource(url.QueryEscape(resource)) + scope.Service(r.Event.Service) + scope.Project(r.Event.Project) + scope.Stage(r.Event.Stage) + + var queryParam api.URIOption + if gitCommitId != "" { + queryParam = api.AppendQuery(url.Values{ + "gitCommitID": []string{gitCommitId}, + }) + } else { + queryParam = api.AppendQuery(url.Values{}) + } + + resourceContent, err := r.ResourceHandler.GetResource(*scope, queryParam) + if err != nil { + return nil, fmt.Errorf("unable to get resouce from keptn: %w", err) + } + + return []byte(resourceContent.ResourceContent), nil +} diff --git a/pkg/keptn/resource_service_test.go b/pkg/keptn/resource_service_test.go new file mode 100644 index 00000000..c79b67c5 --- /dev/null +++ b/pkg/keptn/resource_service_test.go @@ -0,0 +1,63 @@ +package keptn + +import ( + "github.com/golang/mock/gomock" + "github.com/keptn/go-utils/pkg/api/models" + api "github.com/keptn/go-utils/pkg/api/utils" + "github.com/stretchr/testify/require" + keptnfake "keptn-contrib/job-executor-service/pkg/keptn/fake" + "testing" +) + +func TestV1ResourceHandler_GetResource(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockResourceHandler := keptnfake.NewMockKeptnResourceHandler(mockCtrl) + + handler := V1ResourceHandler{ + Event: EventProperties{ + Project: "project", + Stage: "stage", + Service: "service", + }, + ResourceHandler: mockResourceHandler, + } + + tests := []struct { + Test string + GitCommitID string + }{ + { + Test: "With GitCommitID", + GitCommitID: "324723984372948", + }, + { + Test: "Without GitCommitID", + GitCommitID: "", + }, + } + + for _, test := range tests { + t.Run(test.Test, func(t *testing.T) { + expectedBytes := []byte("") + + scope := api.NewResourceScope() + scope.Project("project") + scope.Resource("resource") + scope.Service("service") + scope.Stage("stage") + + mockResourceHandler.EXPECT().GetResource(*scope, gomock.Len(1)).Times(1).Return(&models.Resource{ + Metadata: nil, + ResourceContent: string(expectedBytes), + ResourceURI: nil, + }, nil) + + resource, err := handler.GetResource("resource", test.GitCommitID) + require.NoError(t, err) + require.Equal(t, expectedBytes, resource) + }) + } + +} diff --git a/test/data/e2e/gitcommitid.config.yaml b/test/data/e2e/gitcommitid.config.yaml new file mode 100644 index 00000000..2e3536f3 --- /dev/null +++ b/test/data/e2e/gitcommitid.config.yaml @@ -0,0 +1,12 @@ +apiVersion: v2 +actions: + - name: "Hello World e2e test" + events: + - name: "sh.keptn.event.deployment.triggered" + tasks: + - name: "Greet the world" + image: "alpine" + cmd: + - echo + args: + - "This should not be displayed!" diff --git a/test/e2e/gitcommitid_test.go b/test/e2e/gitcommitid_test.go new file mode 100644 index 00000000..50a8d594 --- /dev/null +++ b/test/e2e/gitcommitid_test.go @@ -0,0 +1,100 @@ +package e2e + +import ( + "github.com/keptn/go-utils/pkg/api/models" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "io/ioutil" + "strings" + "testing" + "time" +) + +func TestGitCommitID(t *testing.T) { + if !isE2ETestingAllowed() { + t.Skip("Skipping TestHelloWorldDeployment, not allowed by environment") + } + + // Setup the E2E test environment + testEnv, err := newTestEnvironment( + "../events/e2e/helloworld.triggered.json", + "../shipyard/e2e/helloworld.deployment.yaml", + "../data/e2e/helloworld.config.yaml", + ) + + require.NoError(t, err) + + err = testEnv.SetupTestEnvironment() + require.NoError(t, err) + + // Make sure project is delete after the tests are completed + defer testEnv.Cleanup() + + // Make sure the integration test is only run for Keptn versions that support the + // gitCommitId parameter for resource queries + if err := testEnv.ShouldRun(">=0.13.0"); err != nil { + t.Skipf("%s\n", err.Error()) + } + + // Send the event to keptn + keptnContext, err := testEnv.API.SendEvent(testEnv.Event) + require.NoError(t, err) + + // Wait for the deployment to be completed + var gitCommitId string + requireWaitForEvent(t, + testEnv.API, + 5*time.Minute, + 1*time.Second, + keptnContext, + "sh.keptn.event.deployment.finished", + func(event *models.KeptnContextExtendedCE) bool { + gitCommitId = event.GitCommitID + return true + }, + ) + + // Upload new job config, that should not get executed when the old git commit id is used: + jobConfigYaml, err := ioutil.ReadFile("../data/e2e/gitcommitid.config.yaml") + require.NoError(t, err) + + err = testEnv.API.CreateJobConfig(testEnv.EventData.Project, testEnv.EventData.Stage, testEnv.EventData.Service, jobConfigYaml) + require.NoError(t, err) + + time.Sleep(1 * time.Second) + + testEnv.Event.GitCommitID = gitCommitId + keptnContext, err = testEnv.API.SendEvent(testEnv.Event) + require.NoError(t, err) + + // Assert that still the old Hello World example is run and not the new one ... + expectedEventData := eventData{ + Project: testEnv.EventData.Project, + Result: "pass", + Service: testEnv.EventData.Service, + Stage: testEnv.EventData.Stage, + Status: "succeeded", + } + + requireWaitForEvent(t, + testEnv.API, + 5*time.Minute, + 1*time.Second, + keptnContext, + "sh.keptn.event.deployment.finished", + func(event *models.KeptnContextExtendedCE) bool { + responseEventData, err := parseKeptnEventData(event) + require.NoError(t, err) + + // If the log contains the Hello world output from the job, we assume that the log + // was correctly read from the job container and set it as expected message + if strings.Contains(responseEventData.Message, "Hello World") { + expectedEventData.Message = responseEventData.Message + } + + assert.Equal(t, expectedEventData, *responseEventData) + return true + }, + ) + +} From 312cdfad934ab550c6a171f3be1dc8ed9d74d79d Mon Sep 17 00:00:00 2001 From: Raphael Ludwig Date: Tue, 5 Jul 2022 13:09:16 +0200 Subject: [PATCH 2/9] refactor: Add auto-generated gomock files Signed-off-by: Raphael Ludwig --- pkg/keptn/fake/keptn_resourcehandler_mock.go | 56 ++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 pkg/keptn/fake/keptn_resourcehandler_mock.go diff --git a/pkg/keptn/fake/keptn_resourcehandler_mock.go b/pkg/keptn/fake/keptn_resourcehandler_mock.go new file mode 100644 index 00000000..9aa12edb --- /dev/null +++ b/pkg/keptn/fake/keptn_resourcehandler_mock.go @@ -0,0 +1,56 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: resource_service.go + +// Package fake is a generated GoMock package. +package fake + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + models "github.com/keptn/go-utils/pkg/api/models" + api "github.com/keptn/go-utils/pkg/api/utils" +) + +// MockKeptnResourceHandler is a mock of KeptnResourceHandler interface. +type MockKeptnResourceHandler struct { + ctrl *gomock.Controller + recorder *MockKeptnResourceHandlerMockRecorder +} + +// MockKeptnResourceHandlerMockRecorder is the mock recorder for MockKeptnResourceHandler. +type MockKeptnResourceHandlerMockRecorder struct { + mock *MockKeptnResourceHandler +} + +// NewMockKeptnResourceHandler creates a new mock instance. +func NewMockKeptnResourceHandler(ctrl *gomock.Controller) *MockKeptnResourceHandler { + mock := &MockKeptnResourceHandler{ctrl: ctrl} + mock.recorder = &MockKeptnResourceHandlerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockKeptnResourceHandler) EXPECT() *MockKeptnResourceHandlerMockRecorder { + return m.recorder +} + +// GetResource mocks base method. +func (m *MockKeptnResourceHandler) GetResource(scope api.ResourceScope, options ...api.URIOption) (*models.Resource, error) { + m.ctrl.T.Helper() + varargs := []interface{}{scope} + for _, a := range options { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetResource", varargs...) + ret0, _ := ret[0].(*models.Resource) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetResource indicates an expected call of GetResource. +func (mr *MockKeptnResourceHandlerMockRecorder) GetResource(scope interface{}, options ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{scope}, options...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetResource", reflect.TypeOf((*MockKeptnResourceHandler)(nil).GetResource), varargs...) +} From 322aa2a50d85093121f722ca66aa02588f1f025a Mon Sep 17 00:00:00 2001 From: Raphael Ludwig Date: Tue, 5 Jul 2022 13:31:00 +0200 Subject: [PATCH 3/9] refactor: Limit new integration test to version >= 0.16.0 Signed-off-by: Raphael Ludwig --- test/e2e/gitcommitid_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/gitcommitid_test.go b/test/e2e/gitcommitid_test.go index 50a8d594..1821aa77 100644 --- a/test/e2e/gitcommitid_test.go +++ b/test/e2e/gitcommitid_test.go @@ -32,7 +32,7 @@ func TestGitCommitID(t *testing.T) { // Make sure the integration test is only run for Keptn versions that support the // gitCommitId parameter for resource queries - if err := testEnv.ShouldRun(">=0.13.0"); err != nil { + if err := testEnv.ShouldRun(">=0.16.0"); err != nil { t.Skipf("%s\n", err.Error()) } From 0574957a22e2df95b2cbd270cb73745ecd20af89 Mon Sep 17 00:00:00 2001 From: Raphael Ludwig Date: Tue, 5 Jul 2022 15:42:53 +0200 Subject: [PATCH 4/9] fix: Integrations tests for >= 0.16.0 Signed-off-by: Raphael Ludwig --- pkg/keptn/config_service.go | 6 +++++- test/e2e/helloworld_test.go | 2 -- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/keptn/config_service.go b/pkg/keptn/config_service.go index 768b22d8..f839320d 100644 --- a/pkg/keptn/config_service.go +++ b/pkg/keptn/config_service.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/keptn/go-utils/pkg/api/models" api "github.com/keptn/go-utils/pkg/api/utils/v2" + "log" "net/url" "os" "strings" @@ -63,7 +64,8 @@ func (k *configServiceImpl) GetKeptnResource(fs afero.Fs, resource string) ([]by } // https://github.com/keptn/keptn/issues/2707 - encodedResource := url.QueryEscape(resource) + // Note: trimming the prefix is necessary if a gitCommitId is used (?) + encodedResource := url.QueryEscape(strings.TrimPrefix(resource, "/")) scope := api.NewResourceScope() scope.Project(k.eventProperties.Project) @@ -82,6 +84,8 @@ func (k *configServiceImpl) GetKeptnResource(fs afero.Fs, resource string) ([]by requestedResource, err := k.resourceHandler.GetResource(context.Background(), *scope, options) + log.Printf("%#v\n%#v\n%#v\n", scope, requestedResource, err) + // return Nil in case resource couldn't be retrieved if err != nil || requestedResource.ResourceContent == "" { return nil, fmt.Errorf("resource not found: %s - %s", resource, err) diff --git a/test/e2e/helloworld_test.go b/test/e2e/helloworld_test.go index 3a081afc..93eb22c9 100644 --- a/test/e2e/helloworld_test.go +++ b/test/e2e/helloworld_test.go @@ -45,8 +45,6 @@ func TestHelloWorldDeployment(t *testing.T) { }, ) - t.Log("Received .started event") - // If the started event was sent by the job executor we wait for a .finished with the following data: expectedEventData := eventData{ Project: testEnv.EventData.Project, From 2e176c4d83ee1df39ae7929a8fcf0a6228c4ec30 Mon Sep 17 00:00:00 2001 From: Raphael Ludwig Date: Tue, 5 Jul 2022 16:03:33 +0200 Subject: [PATCH 5/9] refactor: Apply suggestions from reviewdog Signed-off-by: Raphael Ludwig --- cmd/job-executor-service-initcontainer/main.go | 2 +- pkg/config/reader.go | 8 ++++---- pkg/eventhandler/eventhandlers.go | 14 +++++++------- pkg/eventhandler/eventmapper.go | 4 ++-- pkg/k8sutils/job.go | 6 +++--- pkg/k8sutils/job_test.go | 8 ++++---- pkg/keptn/config_service.go | 13 +++++++------ pkg/keptn/config_service_test.go | 12 ++++++------ pkg/keptn/resource_service.go | 17 +++++++++-------- test/e2e/gitcommitid_test.go | 8 ++++---- 10 files changed, 47 insertions(+), 45 deletions(-) diff --git a/cmd/job-executor-service-initcontainer/main.go b/cmd/job-executor-service-initcontainer/main.go index e566d3d6..ea2401ff 100644 --- a/cmd/job-executor-service-initcontainer/main.go +++ b/cmd/job-executor-service-initcontainer/main.go @@ -125,7 +125,7 @@ func main() { Project: env.Project, Stage: env.Stage, Service: env.Service, - GitCommitId: env.GitCommitID, + GitCommitID: env.GitCommitID, } configService := keptn.NewConfigService(useLocalFileSystem, eventProps, keptnAPI.Resources()) diff --git a/pkg/config/reader.go b/pkg/config/reader.go index 813571d4..d364d294 100644 --- a/pkg/config/reader.go +++ b/pkg/config/reader.go @@ -13,7 +13,7 @@ const jobConfigResourceName = "job/config.yaml" // KeptnResourceService defines the contract used by JobConfigReader to retrieve a resource from keptn (using project, // service, stage from context) type KeptnResourceService interface { - GetResource(resource string, gitCommitId string) ([]byte, error) + GetResource(resource string, gitCommitID string) ([]byte, error) } // JobConfigReader retrieves and parses job configuration from Keptn @@ -25,9 +25,9 @@ type JobConfigReader struct { // Additionally, also the SHA1 hash of the retrieved configuration will be returned. // In case of error retrieving the resource or parsing the yaml it will return (nil, // error) with the original error correctly wrapped in the local one -func (jcr *JobConfigReader) GetJobConfig(gitCommitId string) (*Config, string, error) { +func (jcr *JobConfigReader) GetJobConfig(gitCommitID string) (*Config, string, error) { - resource, err := jcr.Keptn.GetResource(jobConfigResourceName, gitCommitId) + resource, err := jcr.Keptn.GetResource(jobConfigResourceName, gitCommitID) if err != nil { return nil, "", fmt.Errorf("error retrieving job config: %w", err) } @@ -44,7 +44,7 @@ func (jcr *JobConfigReader) GetJobConfig(gitCommitId string) (*Config, string, e return nil, "", fmt.Errorf("error parsing job configuration: %w", err) } - log.Printf("config: SHA3: %s\nGIT :%s\n%s\n", resourceHash, gitCommitId, resource) + log.Printf("config: SHA3: %s\nGIT :%s\n%s\n", resourceHash, gitCommitID, resource) return configuration, resourceHash, nil } diff --git a/pkg/eventhandler/eventhandlers.go b/pkg/eventhandler/eventhandlers.go index 30e4818c..e1a05a74 100644 --- a/pkg/eventhandler/eventhandlers.go +++ b/pkg/eventhandler/eventhandlers.go @@ -38,7 +38,7 @@ type EventMapper interface { // JobConfigReader retrieves the job-executor-service configuration type JobConfigReader interface { - GetJobConfig(gitCommitId string) (*config.Config, string, error) + GetJobConfig(gitCommitID string) (*config.Config, string, error) } // ErrorLogSender is used to send error logs that will appear in Uniform UI @@ -98,12 +98,12 @@ func (eh *EventHandler) HandleEvent() error { log.Printf("CloudEvent %T: %v", eventAsInterface, eventAsInterface) // Get the git commit id from the cloud event (if it exists) and use it to query the job configuration - var gitCommitId string + var gitCommitID string if commitId, ok := eventAsInterface["gitcommitid"]; ok { - gitCommitId, _ = commitId.(string) + gitCommitID, _ = commitId.(string) } - configuration, configHash, err := eh.JobConfigReader.GetJobConfig(gitCommitId) + configuration, configHash, err := eh.JobConfigReader.GetJobConfig(gitCommitID) if err != nil { errorLogErr := eh.ErrorSender.SendErrorLogEvent( @@ -140,14 +140,14 @@ func (eh *EventHandler) HandleEvent() error { eh.Keptn.CloudEvent.Type(), action.Name, ) - eh.startK8sJob(&action, actionIndex, configHash, gitCommitId, eventAsInterface) + eh.startK8sJob(&action, actionIndex, configHash, gitCommitID, eventAsInterface) } } return nil } -func (eh *EventHandler) startK8sJob(action *config.Action, actionIndex int, configHash string, gitCommitId string, +func (eh *EventHandler) startK8sJob(action *config.Action, actionIndex int, configHash string, gitCommitID string, jsonEventData interface{}, ) { @@ -207,7 +207,7 @@ func (eh *EventHandler) startK8sJob(action *config.Action, actionIndex int, conf ActionIndex: actionIndex, TaskIndex: index, JobConfigHash: configHash, - GitCommitId: gitCommitId, + GitCommitID: gitCommitID, } err = eh.K8s.CreateK8sJob( diff --git a/pkg/eventhandler/eventmapper.go b/pkg/eventhandler/eventmapper.go index a13fc7cf..38e6449a 100644 --- a/pkg/eventhandler/eventmapper.go +++ b/pkg/eventhandler/eventmapper.go @@ -26,7 +26,7 @@ func (kcem *KeptnCloudEventMapper) Map(ce cloudevents.Event) (map[string]interfa extension, _ := ce.Context.GetExtension("shkeptncontext") shKeptnContext := extension.(string) - gitcommitidExt, _ := ce.Context.GetExtension("gitcommitid") + gitCommitID, _ := ce.Context.GetExtension("gitcommitid") eventAsInterface := make(map[string]interface{}) eventAsInterface["id"] = ce.ID() @@ -36,7 +36,7 @@ func (kcem *KeptnCloudEventMapper) Map(ce cloudevents.Event) (map[string]interfa eventAsInterface["data"] = eventDataAsInterface eventAsInterface["specversion"] = ce.SpecVersion() eventAsInterface["type"] = ce.Type() - eventAsInterface["gitcommitid"] = gitcommitidExt + eventAsInterface["gitcommitid"] = gitCommitID return eventAsInterface, nil } diff --git a/pkg/k8sutils/job.go b/pkg/k8sutils/job.go index 4554bf0a..5a9b0f71 100644 --- a/pkg/k8sutils/job.go +++ b/pkg/k8sutils/job.go @@ -60,7 +60,7 @@ type JobDetails struct { ActionIndex int TaskIndex int JobConfigHash string - GitCommitId string + GitCommitID string } // JobSettings contains environment variable settings for the job @@ -326,7 +326,7 @@ func (k8s *K8sImpl) CreateK8sJob( }, { Name: "GIT_COMMIT_ID", - Value: jobDetails.GitCommitId, + Value: jobDetails.GitCommitID, }, }, Resources: *jobSettings.DefaultResourceRequirements, @@ -641,7 +641,7 @@ func generateK8sJobLabels(jobDetails JobDetails, jsonEventData interface{}, jesD "app.kubernetes.io/managed-by": jesDeploymentName, "keptn.sh/context": keptnContext, "keptn.sh/event-id": eventID, - "keptn.sh/commitid": jobDetails.GitCommitId, + "keptn.sh/commitid": jobDetails.GitCommitID, "keptn.sh/jes-action": sanitizeLabel(jobDetails.Action.Name), "keptn.sh/jes-task": sanitizeLabel(jobDetails.Task.Name), "keptn.sh/jes-job-confighash": jobDetails.JobConfigHash, diff --git a/pkg/k8sutils/job_test.go b/pkg/k8sutils/job_test.go index 058f1ac0..d69e4047 100644 --- a/pkg/k8sutils/job_test.go +++ b/pkg/k8sutils/job_test.go @@ -1127,9 +1127,9 @@ func TestCreateK8sJobContainsCorrectLabels(t *testing.T) { t.Run(test.name, func(t *testing.T) { jobName := "some-job-name-" + strconv.Itoa(i) - var gitCommitId string + var gitCommitID string if id, ok := test.event["gitCommitid"]; ok { - gitCommitId = id.(string) + gitCommitID = id.(string) } err = k8s.CreateK8sJob( @@ -1146,7 +1146,7 @@ func TestCreateK8sJobContainsCorrectLabels(t *testing.T) { ActionIndex: 0, TaskIndex: 0, JobConfigHash: "", - GitCommitId: gitCommitId, + GitCommitID: gitCommitID, }, &eventData, JobSettings{ @@ -1171,7 +1171,7 @@ func TestCreateK8sJobContainsCorrectLabels(t *testing.T) { "app.kubernetes.io/managed-by": "job-executor-service", "keptn.sh/context": test.event["shkeptncontext"].(string), "keptn.sh/event-id": test.event["id"].(string), - "keptn.sh/commitid": gitCommitId, + "keptn.sh/commitid": gitCommitID, "keptn.sh/jes-action": test.expectedActionName, "keptn.sh/jes-task": test.expectedTaskName, "keptn.sh/jes-job-confighash": "", diff --git a/pkg/keptn/config_service.go b/pkg/keptn/config_service.go index f839320d..5d871d9d 100644 --- a/pkg/keptn/config_service.go +++ b/pkg/keptn/config_service.go @@ -39,11 +39,12 @@ type configServiceImpl struct { resourceHandler V2ResourceHandler } +// EventProperties represents a set of properties of a given cloud event type EventProperties struct { Project string Stage string Service string - GitCommitId string + GitCommitID string } // NewConfigService creates and returns new ConfigService @@ -64,7 +65,7 @@ func (k *configServiceImpl) GetKeptnResource(fs afero.Fs, resource string) ([]by } // https://github.com/keptn/keptn/issues/2707 - // Note: trimming the prefix is necessary if a gitCommitId is used (?) + // Note: trimming the prefix is necessary if a gitCommitID is used (?) encodedResource := url.QueryEscape(strings.TrimPrefix(resource, "/")) scope := api.NewResourceScope() @@ -74,10 +75,10 @@ func (k *configServiceImpl) GetKeptnResource(fs afero.Fs, resource string) ([]by scope.Resource(encodedResource) options := api.ResourcesGetResourceOptions{} - if k.eventProperties.GitCommitId != "" { + if k.eventProperties.GitCommitID != "" { options.URIOptions = []api.URIOption{ api.AppendQuery(url.Values{ - "gitCommitID": []string{k.eventProperties.GitCommitId}, + "gitCommitID": []string{k.eventProperties.GitCommitID}, }), } } @@ -117,7 +118,7 @@ func (k *configServiceImpl) GetAllKeptnResources(fs afero.Fs, resource string) ( return nil, fmt.Errorf("resources not found: %s", err) } - // Go over the resources and fetch the content of each resource with the given gitCommitId: + // Go over the resources and fetch the content of each resource with the given gitCommitID: keptnResources := make(map[string][]byte) for _, serviceResource := range requestedResources { // match against with and without starting slash @@ -129,7 +130,7 @@ func (k *configServiceImpl) GetAllKeptnResources(fs afero.Fs, resource string) ( keptnResourceContent, err := k.GetKeptnResource(fs, *serviceResource.ResourceURI) if err != nil { return nil, fmt.Errorf("could not find file %s for version %s", - *serviceResource.ResourceURI, k.eventProperties.GitCommitId, + *serviceResource.ResourceURI, k.eventProperties.GitCommitID, ) } keptnResources[*serviceResource.ResourceURI] = keptnResourceContent diff --git a/pkg/keptn/config_service_test.go b/pkg/keptn/config_service_test.go index f9ff681e..959a5bb0 100644 --- a/pkg/keptn/config_service_test.go +++ b/pkg/keptn/config_service_test.go @@ -26,11 +26,11 @@ func CreateResourceHandlerMock(t *testing.T) *keptnfake.MockV2ResourceHandler { const service = "carts" const project = "sockshop" const stage = "dev" -const gitCommitId = "6caf78d2c978f7f787" +const gitCommitID = "6caf78d2c978f7f787" func TestGetAllKeptnResources(t *testing.T) { - locustBasic := "/locust/basic.py" - locustFunctional := "/locust/functional.py" + locustBasic := "locust/basic.py" + locustFunctional := "locust/functional.py" resourceHandlerMock := CreateResourceHandlerMock(t) resourceHandlerMock.EXPECT().GetAllServiceResources(gomock.Any(), project, stage, service, gomock.Any()).Times(1).Return( @@ -80,7 +80,7 @@ func TestGetAllKeptnResources(t *testing.T) { Project: project, Stage: stage, Service: service, - GitCommitId: gitCommitId, + GitCommitID: gitCommitID, } configService := NewConfigService(false, event, resourceHandlerMock) @@ -126,7 +126,7 @@ func TestGetAllKeptnResourcesLocal(t *testing.T) { Project: project, Stage: stage, Service: service, - GitCommitId: "", + GitCommitID: "", } configService := NewConfigService(true, event, resourceHandlerMock) @@ -178,7 +178,7 @@ func TestErrorNoDirectoryResourcesLocal(t *testing.T) { Project: project, Stage: stage, Service: service, - GitCommitId: "", + GitCommitID: "", } configService := NewConfigService(true, event, resourceHandlerMock) diff --git a/pkg/keptn/resource_service.go b/pkg/keptn/resource_service.go index 8a43f0e0..b0672c23 100644 --- a/pkg/keptn/resource_service.go +++ b/pkg/keptn/resource_service.go @@ -12,10 +12,11 @@ import ( // getting of resources of a given event type V1ResourceHandler struct { Event EventProperties - ResourceHandler KeptnResourceHandler + ResourceHandler V1KeptnResourceHandler } -func NewV1ResourceHandler(event keptn.EventProperties, handler KeptnResourceHandler) V1ResourceHandler { +// NewV1ResourceHandler creates a new V1ResourceHandler from a given Keptn event and a V1KeptnResourceHandler +func NewV1ResourceHandler(event keptn.EventProperties, handler V1KeptnResourceHandler) V1ResourceHandler { return V1ResourceHandler{ Event: EventProperties{ Project: event.GetProject(), @@ -28,13 +29,13 @@ func NewV1ResourceHandler(event keptn.EventProperties, handler KeptnResourceHand //go:generate mockgen -source=resource_service.go -destination=fake/keptn_resourcehandler_mock.go -package=fake KeptnResourceHandler -// KeptnResourceHandler represents an interface for the api.ResourceHandler struct of the Keptn API -type KeptnResourceHandler interface { +// V1KeptnResourceHandler represents an interface for the api.ResourceHandler struct of the Keptn API +type V1KeptnResourceHandler interface { GetResource(scope api.ResourceScope, options ...api.URIOption) (*models.Resource, error) } -// GetResource returns the contents of a resource for a given gitCommitId -func (r V1ResourceHandler) GetResource(resource string, gitCommitId string) ([]byte, error) { +// GetResource returns the contents of a resource for a given gitCommitID +func (r V1ResourceHandler) GetResource(resource string, gitCommitID string) ([]byte, error) { scope := api.NewResourceScope() scope.Resource(url.QueryEscape(resource)) scope.Service(r.Event.Service) @@ -42,9 +43,9 @@ func (r V1ResourceHandler) GetResource(resource string, gitCommitId string) ([]b scope.Stage(r.Event.Stage) var queryParam api.URIOption - if gitCommitId != "" { + if gitCommitID != "" { queryParam = api.AppendQuery(url.Values{ - "gitCommitID": []string{gitCommitId}, + "gitCommitID": []string{gitCommitID}, }) } else { queryParam = api.AppendQuery(url.Values{}) diff --git a/test/e2e/gitcommitid_test.go b/test/e2e/gitcommitid_test.go index 1821aa77..1a9dbb21 100644 --- a/test/e2e/gitcommitid_test.go +++ b/test/e2e/gitcommitid_test.go @@ -31,7 +31,7 @@ func TestGitCommitID(t *testing.T) { defer testEnv.Cleanup() // Make sure the integration test is only run for Keptn versions that support the - // gitCommitId parameter for resource queries + // gitCommitID parameter for resource queries if err := testEnv.ShouldRun(">=0.16.0"); err != nil { t.Skipf("%s\n", err.Error()) } @@ -41,7 +41,7 @@ func TestGitCommitID(t *testing.T) { require.NoError(t, err) // Wait for the deployment to be completed - var gitCommitId string + var gitCommitID string requireWaitForEvent(t, testEnv.API, 5*time.Minute, @@ -49,7 +49,7 @@ func TestGitCommitID(t *testing.T) { keptnContext, "sh.keptn.event.deployment.finished", func(event *models.KeptnContextExtendedCE) bool { - gitCommitId = event.GitCommitID + gitCommitID = event.GitCommitID return true }, ) @@ -63,7 +63,7 @@ func TestGitCommitID(t *testing.T) { time.Sleep(1 * time.Second) - testEnv.Event.GitCommitID = gitCommitId + testEnv.Event.GitCommitID = gitCommitID keptnContext, err = testEnv.API.SendEvent(testEnv.Event) require.NoError(t, err) From 63b97c7930de9f1883c115e1a9493081f5fefd5c Mon Sep 17 00:00:00 2001 From: Raphael Ludwig Date: Tue, 5 Jul 2022 16:20:06 +0200 Subject: [PATCH 6/9] refactor: Remove debugging output Signed-off-by: Raphael Ludwig --- pkg/keptn/config_service.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/keptn/config_service.go b/pkg/keptn/config_service.go index 5d871d9d..4598f5ed 100644 --- a/pkg/keptn/config_service.go +++ b/pkg/keptn/config_service.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/keptn/go-utils/pkg/api/models" api "github.com/keptn/go-utils/pkg/api/utils/v2" - "log" "net/url" "os" "strings" @@ -85,8 +84,6 @@ func (k *configServiceImpl) GetKeptnResource(fs afero.Fs, resource string) ([]by requestedResource, err := k.resourceHandler.GetResource(context.Background(), *scope, options) - log.Printf("%#v\n%#v\n%#v\n", scope, requestedResource, err) - // return Nil in case resource couldn't be retrieved if err != nil || requestedResource.ResourceContent == "" { return nil, fmt.Errorf("resource not found: %s - %s", resource, err) From 882ced05296ed10b3878d5540946e9e9509aacf3 Mon Sep 17 00:00:00 2001 From: Raphael Ludwig Date: Wed, 6 Jul 2022 08:28:23 +0200 Subject: [PATCH 7/9] refactor: Fix unit tests Signed-off-by: Raphael Ludwig --- pkg/k8sutils/job_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/k8sutils/job_test.go b/pkg/k8sutils/job_test.go index d69e4047..a79e19be 100644 --- a/pkg/k8sutils/job_test.go +++ b/pkg/k8sutils/job_test.go @@ -1128,7 +1128,7 @@ func TestCreateK8sJobContainsCorrectLabels(t *testing.T) { jobName := "some-job-name-" + strconv.Itoa(i) var gitCommitID string - if id, ok := test.event["gitCommitid"]; ok { + if id, ok := test.event["gitcommitid"]; ok { gitCommitID = id.(string) } From e90954b273a2f24484d5c566d70098f8b693ec1d Mon Sep 17 00:00:00 2001 From: Raphael Ludwig Date: Wed, 6 Jul 2022 08:29:47 +0200 Subject: [PATCH 8/9] refactor: Remove debug output Signed-off-by: Raphael Ludwig --- pkg/config/reader.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/config/reader.go b/pkg/config/reader.go index d364d294..9f0a2a30 100644 --- a/pkg/config/reader.go +++ b/pkg/config/reader.go @@ -44,7 +44,5 @@ func (jcr *JobConfigReader) GetJobConfig(gitCommitID string) (*Config, string, e return nil, "", fmt.Errorf("error parsing job configuration: %w", err) } - log.Printf("config: SHA3: %s\nGIT :%s\n%s\n", resourceHash, gitCommitID, resource) - return configuration, resourceHash, nil } From 1230ea6761447146e635b6d73092c4838ace29e4 Mon Sep 17 00:00:00 2001 From: Raphael Ludwig Date: Wed, 6 Jul 2022 09:41:03 +0200 Subject: [PATCH 9/9] refactor: Remove old workaround for url.QueryEscape Signed-off-by: Raphael Ludwig --- pkg/keptn/config_service.go | 6 +----- pkg/keptn/config_service_test.go | 6 ++---- pkg/keptn/resource_service.go | 2 +- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/pkg/keptn/config_service.go b/pkg/keptn/config_service.go index 4598f5ed..cf391a14 100644 --- a/pkg/keptn/config_service.go +++ b/pkg/keptn/config_service.go @@ -63,15 +63,11 @@ func (k *configServiceImpl) GetKeptnResource(fs afero.Fs, resource string) ([]by return k.getKeptnResourceFromLocal(fs, resource) } - // https://github.com/keptn/keptn/issues/2707 - // Note: trimming the prefix is necessary if a gitCommitID is used (?) - encodedResource := url.QueryEscape(strings.TrimPrefix(resource, "/")) - scope := api.NewResourceScope() scope.Project(k.eventProperties.Project) scope.Stage(k.eventProperties.Stage) scope.Service(k.eventProperties.Service) - scope.Resource(encodedResource) + scope.Resource(resource) options := api.ResourcesGetResourceOptions{} if k.eventProperties.GitCommitID != "" { diff --git a/pkg/keptn/config_service_test.go b/pkg/keptn/config_service_test.go index 959a5bb0..b4cfd0e3 100644 --- a/pkg/keptn/config_service_test.go +++ b/pkg/keptn/config_service_test.go @@ -18,8 +18,6 @@ import ( func CreateResourceHandlerMock(t *testing.T) *keptnfake.MockV2ResourceHandler { mockCtrl := gomock.NewController(t) - defer mockCtrl.Finish() - return keptnfake.NewMockV2ResourceHandler(mockCtrl) } @@ -52,7 +50,7 @@ func TestGetAllKeptnResources(t *testing.T) { scope1.Project(project) scope1.Stage(stage) scope1.Service(service) - scope1.Resource(url.QueryEscape(locustBasic)) + scope1.Resource(locustBasic) resourceHandlerMock.EXPECT().GetResource(gomock.Any(), *scope1, gomock.Any()).Times(1).Return( &models.Resource{ @@ -66,7 +64,7 @@ func TestGetAllKeptnResources(t *testing.T) { scope2.Project(project) scope2.Stage(stage) scope2.Service(service) - scope2.Resource(url.QueryEscape(locustFunctional)) + scope2.Resource(locustFunctional) resourceHandlerMock.EXPECT().GetResource(gomock.Any(), *scope2, gomock.Any()).Times(1).Return( &models.Resource{ diff --git a/pkg/keptn/resource_service.go b/pkg/keptn/resource_service.go index b0672c23..0da47590 100644 --- a/pkg/keptn/resource_service.go +++ b/pkg/keptn/resource_service.go @@ -37,10 +37,10 @@ type V1KeptnResourceHandler interface { // GetResource returns the contents of a resource for a given gitCommitID func (r V1ResourceHandler) GetResource(resource string, gitCommitID string) ([]byte, error) { scope := api.NewResourceScope() - scope.Resource(url.QueryEscape(resource)) scope.Service(r.Event.Service) scope.Project(r.Event.Project) scope.Stage(r.Event.Stage) + scope.Resource(resource) var queryParam api.URIOption if gitCommitID != "" {