From 9d43143569cce533ec7cfd2c3cdfa16cb7444858 Mon Sep 17 00:00:00 2001 From: Kirill Batalin Date: Mon, 24 Apr 2023 17:57:18 +0100 Subject: [PATCH] fix(pipelineTemplateSave): Take tags from template files (#355) * fix(pipelineTemplateSave): Take tags from template files Spin ignores tags from pipeline template files, and it causes errors during saving. Example of the problem: 1. Create a template with a tag "stable" inside 2. Save it via `spin pt save`. Don't pass the tag to the command, because it already exists in the code 3. Front50 parses the template and finds the tag inside: https://github.com/spinnaker/front50/blob/master/front50-web/src/main/java/com/netflix/spinnaker/front50/controllers/V2PipelineTemplateController.java#L94 4. Front50 saves the template with the tag 5. Edit the code of the template and save it again 6. Spin tries to find a template in Front50 without tags (because we don't pass the tag via arguments) 7. Spin can't find the template and invokes "create" method instead of "update" 8. Front50 throws an DuplicateEntityException because a template with the id already exists. Validation code: https://github.com/spinnaker/front50/blob/master/front50-web/src/main/java/com/netflix/spinnaker/front50/controllers/V2PipelineTemplateController.java#L115 Front50 takes a tag from query parameters as a priority. And only if the tag is not in the query parameters, it tries to take it from the template source code. https://github.com/spinnaker/front50/blob/master/front50-web/src/main/java/com/netflix/spinnaker/front50/controllers/V2PipelineTemplateController.java#L93 I reproduced a similar logic in Spin * fix comment in tests --- cmd/pipeline-template/save.go | 2 + cmd/pipeline-template/save_test.go | 154 +++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+) diff --git a/cmd/pipeline-template/save.go b/cmd/pipeline-template/save.go index d24e20da..194e1c12 100644 --- a/cmd/pipeline-template/save.go +++ b/cmd/pipeline-template/save.go @@ -84,6 +84,8 @@ func savePipelineTemplate(cmd *cobra.Command, options *saveOptions) error { getQueryParam := &gate.V2PipelineTemplatesControllerApiGetUsingGET2Opts{} if options.tag != "" { getQueryParam.Tag = optional.NewString(options.tag) + } else if tag, exists := templateJson["tag"]; exists && tag.(string) != "" { + getQueryParam.Tag = optional.NewString(tag.(string)) } _, resp, queryErr := options.GateClient.V2PipelineTemplatesControllerApi.GetUsingGET2(options.GateClient.Context, templateId, getQueryParam) diff --git a/cmd/pipeline-template/save_test.go b/cmd/pipeline-template/save_test.go index 44a91cd5..7fc1783a 100644 --- a/cmd/pipeline-template/save_test.go +++ b/cmd/pipeline-template/save_test.go @@ -133,6 +133,70 @@ func TestPipelineTemplateSave_update(t *testing.T) { util.TestPrettyJsonDiff(t, "save request body", expected, recieved) } +func TestPipelineTemplateSave_updatetagfromfile(t *testing.T) { + saveBuffer := new(bytes.Buffer) + method := new(string) + ts := testGatePipelineTemplateUpdateTagSuccess(saveBuffer, method, "stable") + defer ts.Close() + + tempFile := tempPipelineTemplateFile(testPipelineTemplateWithTagJsonStr) + if tempFile == nil { + t.Fatal("Could not create temp pipeline template file.") + } + defer os.Remove(tempFile.Name()) + + rootCmd, rootOpts := cmd.NewCmdRoot(ioutil.Discard, ioutil.Discard) + rootCmd.AddCommand(NewPipelineTemplateCmd(rootOpts)) + + args := []string{"pipeline-template", "save", "--file", tempFile.Name(), "--gate-endpoint", ts.URL} + rootCmd.SetArgs(args) + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Command failed with: %s", err) + } + + expected := strings.TrimSpace(testPipelineTemplateWithTagJsonStr) + recieved := saveBuffer.Bytes() + util.TestPrettyJsonDiff(t, "save request body", expected, recieved) + + // Verify that the commad used the tag from the file + if *method != "update" { + t.Fatalf("Expected 'update' request, got %s", *method) + } +} + +func TestPipelineTemplateSave_taginargumentsandfile(t *testing.T) { + saveBuffer := new(bytes.Buffer) + method := new(string) + ts := testGatePipelineTemplateUpdateTagSuccess(saveBuffer, method, "test") + defer ts.Close() + + tempFile := tempPipelineTemplateFile(testPipelineTemplateWithTagJsonStr) + if tempFile == nil { + t.Fatal("Could not create temp pipeline template file.") + } + defer os.Remove(tempFile.Name()) + + rootCmd, rootOpts := cmd.NewCmdRoot(ioutil.Discard, ioutil.Discard) + rootCmd.AddCommand(NewPipelineTemplateCmd(rootOpts)) + + args := []string{"pipeline-template", "save", "--file", tempFile.Name(), "--tag", "test", "--gate-endpoint", ts.URL} + rootCmd.SetArgs(args) + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Command failed with: %s", err) + } + + expected := strings.TrimSpace(testPipelineTemplateWithTagJsonStr) + recieved := saveBuffer.Bytes() + util.TestPrettyJsonDiff(t, "save request body", expected, recieved) + + // Tag in arguments should take precedence over tag in file + if *method != "update" { + t.Fatalf("Expected 'update' request, got %s", *method) + } +} + func TestPipelineTemplateSave_updatetag(t *testing.T) { saveBuffer := new(bytes.Buffer) ts := testGatePipelineTemplateUpdateSuccess(saveBuffer) @@ -301,6 +365,39 @@ func tempPipelineTemplateFile(pipelineContent string) *os.File { return tempFile } +// testGatePipelineTemplateUpdateTagSuccess spins up a local http server that we will configure the GateClient +// to direct requests to. +// Responds with OK to indicate a pipeline template with an expected tag exists. +// Responds with 404 NotFound to indicate a pipeline template with an expected tag doesn't exist. +// Accepts POST calls for create and update requests. +// Writes used method and request body to buffer for testing. +func testGatePipelineTemplateUpdateTagSuccess(buffer io.Writer, method *string, expectedTag string) *httptest.Server { + mux := util.TestGateMuxWithVersionHandler() + mux.Handle( + "/v2/pipelineTemplates/update/testSpelTemplate", + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + *method = "update" + util.NewTestBufferHandlerFunc(http.MethodPost, buffer, http.StatusOK, "").ServeHTTP(w, r) + }), + ) + mux.Handle( + "/v2/pipelineTemplates/create", + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + *method = "create" + util.NewTestBufferHandlerFunc(http.MethodPost, buffer, http.StatusAccepted, "").ServeHTTP(w, r) + }), + ) + // Return that we found an MPT if a tag from the request equals to expectedTag. + mux.Handle("/v2/pipelineTemplates/testSpelTemplate", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("tag") == expectedTag { + w.WriteHeader(http.StatusOK) + } else { + w.WriteHeader(http.StatusNotFound) + } + })) + return httptest.NewServer(mux) +} + // testGatePipelineTemplateUpdateSuccess spins up a local http server that we will configure the GateClient // to direct requests to. Responds with OK to indicate a pipeline template exists, // and Accepts POST calls. @@ -501,6 +598,63 @@ const testPipelineTemplateJsonStr = ` } ` +const testPipelineTemplateWithTagJsonStr = ` +{ + "id": "testSpelTemplate", + "lastModifiedBy": "anonymous", + "metadata": { + "description": "A generic application bake and tag pipeline.", + "name": "Default Bake and Tag", + "owner": "example@example.com", + "scopes": [ + "global" + ] + }, + "pipeline": { + "description": "", + "keepWaitingPipelines": false, + "lastModifiedBy": "anonymous", + "limitConcurrent": true, + "notifications": [], + "parameterConfig": [], + "stages": [ + { + "name": "My Wait Stage", + "refId": "wait1", + "requisiteStageRefIds": [], + "type": "wait", + "waitTime": "${ templateVariables.waitTime }" + } + ], + "triggers": [ + { + "attributeConstraints": {}, + "enabled": true, + "payloadConstraints": {}, + "pubsubSystem": "google", + "source": "jake", + "subscription": "super-why", + "subscriptionName": "super-why", + "type": "pubsub" + } + ], + "updateTs": "1543509523663" + }, + "protect": false, + "schema": "v2", + "tag": "stable", + "updateTs": "1544475186050", + "variables": [ + { + "defaultValue": 42, + "description": "The time a wait stage shall pauseth", + "name": "waitTime", + "type": "int" + } + ] +} +` + const testPipelineTemplateYamlStr = ` id: testSpelTemplate lastModifiedBy: anonymous