From d6d96b58cbbfae864c29224c2ff775285d218689 Mon Sep 17 00:00:00 2001 From: Evan Naderi <61552481+evannaderi@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:27:18 -0400 Subject: [PATCH 1/2] fix: accept newline-separated manifest path lists MANIFESTPATH (Azure Pipelines) and DEPLOYMENTMANIFESTPATH (GitHub Actions) feed the manifests input of KubernetesManifest@1 and Azure/k8s-deploy, which take multiple paths separated by newlines. The dirPath kind rejects newlines, so callers can no longer generate pipelines that deploy more than one manifest path. Add a dirPathList kind that validates each newline-separated entry with dirPathValidator, and use it for both variables. Both templates already render these values with jsonString, so newlines stay inside a single YAML scalar. A template test asserts every dirPathList variable is rendered through jsonString. --- pkg/config/draftconfig_template_test.go | 49 +++++++++++++++++ pkg/config/validators/validators.go | 14 +++++ pkg/config/validators/validators_test.go | 19 +++++++ pkg/handlers/template_test.go | 57 ++++++++++++++++++++ template/azurePipelines/manifests/draft.yaml | 2 +- template/workflows/manifests/draft.yaml | 2 +- 6 files changed, 141 insertions(+), 2 deletions(-) diff --git a/pkg/config/draftconfig_template_test.go b/pkg/config/draftconfig_template_test.go index 35d1c87b5..f4e6fcf95 100644 --- a/pkg/config/draftconfig_template_test.go +++ b/pkg/config/draftconfig_template_test.go @@ -3,6 +3,7 @@ package config import ( "fmt" "io/fs" + "path" "regexp" "slices" "strings" @@ -41,6 +42,7 @@ var validVariableKinds = map[string]bool{ "containerImageVersion": true, "clusterResourceType": true, "dirPath": true, + "dirPathList": true, "dockerFileName": true, "envVarMap": true, "fileName": true, @@ -135,6 +137,53 @@ func TestShippedVariableValuesPassValidation(t *testing.T) { assert.Nil(t, err) } +func TestDirPathListVariablesAreJSONEncoded(t *testing.T) { + checked := 0 + err := fs.WalkDir(template.Templates, ".", func(configPath string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() || !strings.EqualFold(d.Name(), draftConfigFile) { + return nil + } + + currTemplate, err := NewConfigFromFS(template.Templates, configPath) + if err != nil { + return err + } + + templateDir := path.Dir(configPath) + for _, variable := range currTemplate.Variables { + if variable.Kind != "dirPathList" { + continue + } + reference := regexp.MustCompile(`GetVariableValue\s+"` + regexp.QuoteMeta(variable.Name) + `"\s*(\|\s*jsonString\b)?`) + err := fs.WalkDir(template.Templates, templateDir, func(filePath string, d fs.DirEntry, err error) error { + if err != nil || d.IsDir() || filePath == configPath { + return err + } + content, err := fs.ReadFile(template.Templates, filePath) + if err != nil { + return err + } + for _, match := range reference.FindAllStringSubmatch(string(content), -1) { + checked++ + if match[1] == "" { + t.Errorf("%s renders %s (kind dirPathList) without jsonString", filePath, variable.Name) + } + } + return nil + }) + if err != nil { + return err + } + } + return nil + }) + assert.Nil(t, err) + assert.NotZero(t, checked, "expected at least one dirPathList variable reference") +} + func loadTemplatesWithValidation() error { regexp := regexp.MustCompile(alphaNumUnderscoreHyphen) return fs.WalkDir(template.Templates, ".", func(path string, d fs.DirEntry, err error) error { diff --git a/pkg/config/validators/validators.go b/pkg/config/validators/validators.go index cee767385..77f8135da 100644 --- a/pkg/config/validators/validators.go +++ b/pkg/config/validators/validators.go @@ -66,6 +66,8 @@ func GetValidator(variableKind string) func(string) error { return containerImageVersionValidator case "dirPath": return dirPathValidator + case "dirPathList": + return dirPathListValidator case "envVarMap": return keyValueMapValidator case "fileName": @@ -217,6 +219,18 @@ func dirPathValidator(input string) error { return nil } +func dirPathListValidator(input string) error { + if input == "" { + return fmt.Errorf("invalid directory path list: must not be empty") + } + for _, entry := range strings.Split(input, "\n") { + if err := dirPathValidator(entry); err != nil { + return fmt.Errorf("invalid directory path list: %w", err) + } + } + return nil +} + func repositoryBranchValidator(input string) error { if input == "" { return fmt.Errorf("invalid repository branch: must not be empty") diff --git a/pkg/config/validators/validators_test.go b/pkg/config/validators/validators_test.go index fd12f4dd7..fdb4c7e2e 100644 --- a/pkg/config/validators/validators_test.go +++ b/pkg/config/validators/validators_test.go @@ -162,6 +162,25 @@ func TestDirPathValidator(t *testing.T) { } } +func TestDirPathListValidator(t *testing.T) { + for _, input := range []string{"./manifests/*", "./a.yaml\n./b/c.yaml", "src/my app\n資料/服務"} { + assert.Nil(t, GetValidator("dirPathList")(input), "expected %q to be valid", input) + } + + for _, input := range []string{ + "", + "./a.yaml\n", + "\n./a.yaml", + "./a.yaml\n\n./b.yaml", + "./a.yaml\r\n./b.yaml", + "./a.yaml\n$(touch pwned)", + "./a.yaml\n-rf", + "./a.yaml\n\"quoted\"", + } { + assert.NotNil(t, GetValidator("dirPathList")(input), "expected %q to be rejected", input) + } +} + func TestRepositoryBranchValidator(t *testing.T) { for _, input := range []string{"main", "feature/security-fix", "release-1.2"} { assert.Nil(t, GetValidator("repositoryBranch")(input), "expected %q to be valid", input) diff --git a/pkg/handlers/template_test.go b/pkg/handlers/template_test.go index 0b1504e7d..efcd79e97 100644 --- a/pkg/handlers/template_test.go +++ b/pkg/handlers/template_test.go @@ -224,3 +224,60 @@ func TestPythonEntrypointJSONEncoding(t *testing.T) { }) } } + +func TestManifestPathListRendersAsOneValue(t *testing.T) { + paths := "./manifests/deployment.yaml\n./manifests/service.yaml\n - script: echo injected" + + testCases := []struct { + name string + template string + output string + values map[string]string + lookup func(map[string]any) any + }{ + { + name: "azure pipeline", + template: "azure-pipeline-manifests", + output: ".pipelines/azure-kubernetes-service.yaml", + values: map[string]string{ + "ARMSERVICECONNECTION": "conn", "AZURECONTAINERREGISTRY": "registry", + "CONTAINERNAME": "app", "CLUSTERRESOURCEGROUP": "cluster-rg", + "ACRRESOURCEGROUP": "acr-rg", "CLUSTERNAME": "cluster", + "CLUSTERRESOURCETYPE": "Microsoft.ContainerService/managedClusters", + "MANIFESTPATH": paths, + }, + lookup: func(doc map[string]any) any { return doc["variables"].(map[string]any)["manifestPath"] }, + }, + { + name: "github workflow", + template: "github-workflow-manifests", + output: ".github/workflows/azure-kubernetes-service.yml", + values: map[string]string{ + "WORKFLOWNAME": "deploy", "BRANCHNAME": "main", + "ACRRESOURCEGROUP": "acr-rg", "AZURECONTAINERREGISTRY": "registry", + "CONTAINERNAME": "app", "CLUSTERRESOURCEGROUP": "cluster-rg", + "CLUSTERNAME": "cluster", "DOCKERFILE": "./Dockerfile", + "BUILDCONTEXTPATH": ".", "NAMESPACE": "default", + "DEPLOYMENTMANIFESTPATH": paths, + }, + lookup: func(doc map[string]any) any { return doc["env"].(map[string]any)["DEPLOYMENT_MANIFEST_PATH"] }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + writer := &writers.FileMapWriter{FileMap: make(map[string][]byte)} + tpl, err := GetTemplate(testCase.template, "", ".", writer) + assert.Nil(t, err) + for name, value := range testCase.values { + tpl.Config.SetVariable(name, value) + } + assert.Nil(t, tpl.Generate()) + + var doc map[string]any + assert.Nil(t, yaml.Unmarshal(writer.FileMap[testCase.output], &doc)) + assert.Equal(t, paths, testCase.lookup(doc)) + assert.NotContains(t, string(writer.FileMap[testCase.output]), "\n - script: echo injected") + }) + } +} diff --git a/template/azurePipelines/manifests/draft.yaml b/template/azurePipelines/manifests/draft.yaml index 8459c1608..700755c52 100644 --- a/template/azurePipelines/manifests/draft.yaml +++ b/template/azurePipelines/manifests/draft.yaml @@ -57,7 +57,7 @@ variables: versions: ">=0.0.1" - name: "MANIFESTPATH" type: "string" - kind: "dirPath" + kind: "dirPathList" default: disablePrompt: true value: "./manifests/*" diff --git a/template/workflows/manifests/draft.yaml b/template/workflows/manifests/draft.yaml index bc3b715f6..c4c622e86 100644 --- a/template/workflows/manifests/draft.yaml +++ b/template/workflows/manifests/draft.yaml @@ -43,7 +43,7 @@ variables: versions: ">=0.0.1" - name: "DEPLOYMENTMANIFESTPATH" type: "string" - kind: "dirPath" + kind: "dirPathList" default: disablePrompt: true value: "./manifests" From 4a8ef18e5cb023ab5a3ae644cdf65b79838a7c5c Mon Sep 17 00:00:00 2001 From: Evan Naderi <61552481+evannaderi@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:32:45 -0400 Subject: [PATCH 2/2] address review comments --- pkg/config/validators/validators.go | 3 +++ pkg/config/validators/validators_test.go | 3 +++ pkg/handlers/template_test.go | 3 +-- template/azurePipelines/manifests/draft.yaml | 2 +- template/workflows/manifests/draft.yaml | 2 +- 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/config/validators/validators.go b/pkg/config/validators/validators.go index 77f8135da..5cf24e869 100644 --- a/pkg/config/validators/validators.go +++ b/pkg/config/validators/validators.go @@ -224,6 +224,9 @@ func dirPathListValidator(input string) error { return fmt.Errorf("invalid directory path list: must not be empty") } for _, entry := range strings.Split(input, "\n") { + if strings.TrimSpace(entry) != entry { + return fmt.Errorf("invalid directory path list: entry %q has leading or trailing whitespace", entry) + } if err := dirPathValidator(entry); err != nil { return fmt.Errorf("invalid directory path list: %w", err) } diff --git a/pkg/config/validators/validators_test.go b/pkg/config/validators/validators_test.go index fdb4c7e2e..0731ade17 100644 --- a/pkg/config/validators/validators_test.go +++ b/pkg/config/validators/validators_test.go @@ -175,6 +175,9 @@ func TestDirPathListValidator(t *testing.T) { "./a.yaml\r\n./b.yaml", "./a.yaml\n$(touch pwned)", "./a.yaml\n-rf", + " ./a.yaml", + "./a.yaml \n./b.yaml", + "./a.yaml\n ./b.yaml", "./a.yaml\n\"quoted\"", } { assert.NotNil(t, GetValidator("dirPathList")(input), "expected %q to be rejected", input) diff --git a/pkg/handlers/template_test.go b/pkg/handlers/template_test.go index efcd79e97..3000734b4 100644 --- a/pkg/handlers/template_test.go +++ b/pkg/handlers/template_test.go @@ -226,7 +226,7 @@ func TestPythonEntrypointJSONEncoding(t *testing.T) { } func TestManifestPathListRendersAsOneValue(t *testing.T) { - paths := "./manifests/deployment.yaml\n./manifests/service.yaml\n - script: echo injected" + paths := "./manifests/deployment.yaml\n./manifests/service.yaml\n./manifests/ingress.yaml" testCases := []struct { name string @@ -277,7 +277,6 @@ func TestManifestPathListRendersAsOneValue(t *testing.T) { var doc map[string]any assert.Nil(t, yaml.Unmarshal(writer.FileMap[testCase.output], &doc)) assert.Equal(t, paths, testCase.lookup(doc)) - assert.NotContains(t, string(writer.FileMap[testCase.output]), "\n - script: echo injected") }) } } diff --git a/template/azurePipelines/manifests/draft.yaml b/template/azurePipelines/manifests/draft.yaml index 700755c52..9c41b90e6 100644 --- a/template/azurePipelines/manifests/draft.yaml +++ b/template/azurePipelines/manifests/draft.yaml @@ -61,7 +61,7 @@ variables: default: disablePrompt: true value: "./manifests/*" - description: "the path to the Kubernetes deployment manifest" + description: "one or more newline-separated paths to Kubernetes deployment manifests" versions: ">=0.0.1" - name: "NAMESPACE" type: "string" diff --git a/template/workflows/manifests/draft.yaml b/template/workflows/manifests/draft.yaml index c4c622e86..9e85105e7 100644 --- a/template/workflows/manifests/draft.yaml +++ b/template/workflows/manifests/draft.yaml @@ -47,7 +47,7 @@ variables: default: disablePrompt: true value: "./manifests" - description: "the path to the Kubernetes deployment manifest" + description: "one or more newline-separated paths to Kubernetes deployment manifests" versions: ">=0.0.1" - name: "DOCKERFILE" type: "string"