From b4f23537a22137b3cb0cd154e9b9abddca2a80f7 Mon Sep 17 00:00:00 2001 From: Kumar Piyush Date: Wed, 17 Jun 2026 01:01:26 +0530 Subject: [PATCH 01/10] Add PathPattern (directory discovery) to ArtifactGenerator Signed-off-by: Kumar Piyush --- api/v1beta1/artifactgenerator_types.go | 10 +- ...tensions.fluxcd.io_artifactgenerators.yaml | 10 +- docs/spec/v1beta1/artifactgenerators.md | 48 ++- .../artifactgenerator_controller.go | 28 +- .../artifactgenerator_controller_test.go | 190 ++++++++++ .../artifactgenerator_drift_test.go | 2 +- .../artifactgenerator_pathpattern.go | 216 +++++++++++ .../artifactgenerator_pathpattern_test.go | 357 ++++++++++++++++++ 8 files changed, 851 insertions(+), 10 deletions(-) create mode 100644 internal/controller/artifactgenerator_pathpattern.go create mode 100644 internal/controller/artifactgenerator_pathpattern_test.go diff --git a/api/v1beta1/artifactgenerator_types.go b/api/v1beta1/artifactgenerator_types.go index 0a9f65ac..dd648a87 100644 --- a/api/v1beta1/artifactgenerator_types.go +++ b/api/v1beta1/artifactgenerator_types.go @@ -68,6 +68,14 @@ type ArtifactGeneratorSpec struct { // +required Sources []SourceReference `json:"sources"` + // PathPattern specifies a directory traversal pattern to match within the sources. + // The format is "@/". Named captures in the pattern (e.g. "{app}") + // can be used as Go template variables in OutputArtifacts fields. + // +kubebuilder:validation:Pattern="^@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)$" + // +kubebuilder:validation:MaxLength=1024 + // +optional + PathPattern string `json:"pathPattern,omitempty"` + // OutputArtifacts is a list of output artifacts to be generated. // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=1000 @@ -110,7 +118,7 @@ type SourceReference struct { // generated by the ArtifactGenerator. type OutputArtifact struct { // Name is the name of the generated artifact. - // +kubebuilder:validation:Pattern="^[a-z0-9]([a-z0-9-]*[a-z0-9])?$" + // +kubebuilder:validation:Pattern="^[a-zA-Z0-9{]([a-zA-Z0-9\\-\\{\\}\\.\\s]*[a-zA-Z0-9}])?$" // +kubebuilder:validation:MaxLength=253 // +required Name string `json:"name"` diff --git a/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml b/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml index 92791680..3738f93e 100644 --- a/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml +++ b/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml @@ -114,7 +114,7 @@ spec: name: description: Name is the name of the generated artifact. maxLength: 253 - pattern: ^[a-z0-9]([a-z0-9-]*[a-z0-9])?$ + pattern: ^[a-zA-Z0-9{]([a-zA-Z0-9\-\{\}\.\s]*[a-zA-Z0-9}])?$ type: string originRevision: description: |- @@ -159,6 +159,14 @@ spec: description: Labels to be added to the object's metadata. type: object type: object + pathPattern: + description: |- + PathPattern specifies a directory traversal pattern to match within the sources. + The format is "@/". Named captures in the pattern (e.g. "{app}") + can be used as Go template variables in OutputArtifacts fields. + maxLength: 1024 + pattern: ^@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)$ + type: string sources: description: |- Sources is a list of references to the Flux source-controller diff --git a/docs/spec/v1beta1/artifactgenerators.md b/docs/spec/v1beta1/artifactgenerators.md index 3ed14746..6522a579 100644 --- a/docs/spec/v1beta1/artifactgenerators.md +++ b/docs/spec/v1beta1/artifactgenerators.md @@ -227,13 +227,59 @@ spec: Sources are watched for changes, and when any source is updated, the controller will regenerate the affected artifacts automatically. +### Path Pattern (Directory Discovery) + +The `.spec.pathPattern` field allows for dynamic, path-based discovery of artifacts. When specified, the controller will scan the referenced source for directories matching the pattern and dynamically generate one ExternalArtifact for each matched directory. + +- `pathPattern` (optional): Specifies a directory traversal pattern within a source in the format `@/`. + Named captures in the pattern (e.g., `{app}`, `{env}`) can be used as Go template variables (`{{ .app }}`, `{{ .env }}`) inside the `artifacts` fields. + +When `pathPattern` is used, the generated ExternalArtifacts will automatically have their labels populated with the extracted capture variables. + +```yaml +spec: + sources: + - alias: monorepo + kind: GitRepository + name: my-monorepo + pathPattern: "@monorepo/apps/{app}/envs/{env}" + artifacts: + - name: "{{ .app }}-{{ .env }}" + copy: + - from: "@monorepo/apps/{{ .app }}/envs/{{ .env }}/**" + to: "@artifact/" +``` + +#### Directory Name Constraints + +Directory names matched by path pattern captures must comply with +[Kubernetes label value restrictions](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-semantics): + +- Must be 63 characters or fewer +- Must begin and end with an alphanumeric character (`[a-zA-Z0-9]`) +- May contain dashes (`-`), underscores (`_`), dots (`.`), and alphanumeric characters + +The controller automatically lowercases all captured values before using them +in artifact names and labels. This means a directory named `Auth` will produce +an artifact with `app=auth`. + +**Note:** If two directories differ only in case (e.g., `apps/Auth/` and `apps/auth/`), +they will resolve to the same artifact name after lowercasing. In this case, the +controller will stall the reconciliation with an error indicating the collision. + +If any captured directory name does not conform to the label value restrictions +(e.g., names starting with a dot like `.hidden`, containing spaces, or exceeding +63 characters), the reconciliation will fail with a terminal error that includes +the `pathPattern` and the invalid value. + ### Artifacts The `.spec.artifacts` field defines the list of ExternalArtifacts to be generated from the sources. +When `pathPattern` is defined, the artifacts act as a template for each matched directory. Each artifact must specify: - `name` (required): The name of the generated ExternalArtifact resource. It must be unique in the context - of the ArtifactGenerator and must conform to Kubernetes resource naming conventions. + of the ArtifactGenerator and must conform to Kubernetes resource naming conventions. Supports Go template variables if `pathPattern` is used. - `copy` (required): A list of copy operations to perform from sources to the artifact. - `revision` (optional): A specific source revision to use in the format `@alias`. If not specified, the revision is automatically computed as `latest@` based on the artifact content. diff --git a/internal/controller/artifactgenerator_controller.go b/internal/controller/artifactgenerator_controller.go index 415a83c6..79d67b62 100644 --- a/internal/controller/artifactgenerator_controller.go +++ b/internal/controller/artifactgenerator_controller.go @@ -193,14 +193,27 @@ func (r *ArtifactGeneratorReconciler) reconcile(ctx context.Context, return ctrl.Result{RequeueAfter: r.DependencyRequeueInterval}, nil } - // Prepare a slice to hold the references to the created ExternalArtifact objects. - eaRefs := make([]swapi.ExternalArtifactReference, 0, len(obj.Spec.OutputArtifacts)) - // Build the artifacts and reconcile the ExternalArtifact objects. // The artifacts will be stored in the storage under the following path: // ////.tar.gz artifactBuilder := builder.New(r.Storage) - for _, oa := range obj.Spec.OutputArtifacts { + + reqs, err := buildArtifactRequests(obj, localSources) + if err != nil { + msg := fmt.Sprintf("failed to expand path pattern: %s", err.Error()) + gotkconditions.MarkFalse(obj, + gotkmeta.ReadyCondition, + gotkmeta.BuildFailedReason, + "%s", msg) + r.Event(obj, corev1.EventTypeWarning, gotkmeta.BuildFailedReason, msg) + return ctrl.Result{}, err + } + + // Prepare a slice to hold the references to the created ExternalArtifact objects. + eaRefs := make([]swapi.ExternalArtifactReference, 0, len(reqs)) + + for _, req := range reqs { + oa := req.OutputArtifact // Build the artifact using the local sources. artifact, err := artifactBuilder.Build(ctx, &oa, localSources, obj.Namespace, tmpDir) if err != nil { @@ -219,7 +232,7 @@ func (r *ArtifactGeneratorReconciler) reconcile(ctx context.Context, // Reconcile the ExternalArtifact corresponding to the built artifact. // The ExternalArtifact will reference the artifact stored in the storage backend. // If the ExternalArtifact already exists, its status will be updated with the new artifact details. - eaRef, err := r.reconcileExternalArtifact(ctx, obj, &oa, artifact) + eaRef, err := r.reconcileExternalArtifact(ctx, obj, &oa, artifact, req.Labels) if err != nil { msg := fmt.Sprintf("%s reconcile failed: %s", oa.Name, err.Error()) gotkconditions.MarkFalse(obj, @@ -423,7 +436,8 @@ func (r *ArtifactGeneratorReconciler) fetchSources(ctx context.Context, func (r *ArtifactGeneratorReconciler) reconcileExternalArtifact(ctx context.Context, obj *swapi.ArtifactGenerator, outputArtifact *swapi.OutputArtifact, - artifact *gotkmeta.Artifact) (*swapi.ExternalArtifactReference, error) { + artifact *gotkmeta.Artifact, + dynamicLabels map[string]string) (*swapi.ExternalArtifactReference, error) { log := ctrl.LoggerFrom(ctx) // Prepare labels for the ExternalArtifact with the managed-by and generator labels. @@ -438,6 +452,8 @@ func (r *ArtifactGeneratorReconciler) reconcileExternalArtifact(ctx context.Cont } } + maps.Copy(labels, dynamicLabels) + labels["app.kubernetes.io/managed-by"] = r.ControllerName labels[swapi.ArtifactGeneratorLabel] = string(obj.GetUID()) diff --git a/internal/controller/artifactgenerator_controller_test.go b/internal/controller/artifactgenerator_controller_test.go index 28299838..ea5566bb 100644 --- a/internal/controller/artifactgenerator_controller_test.go +++ b/internal/controller/artifactgenerator_controller_test.go @@ -39,6 +39,8 @@ import ( gotktestsrv "github.com/fluxcd/pkg/testserver" sourcev1 "github.com/fluxcd/source-controller/api/v1" + gotkfetch "github.com/fluxcd/pkg/http/fetch" + gotktar "github.com/fluxcd/pkg/tar" swapi "github.com/fluxcd/source-watcher/api/v2/v1beta1" ) @@ -848,3 +850,191 @@ func findArtifactsInStorage(namespace string) ([]string, error) { } return artifacts, err } + +func TestArtifactGeneratorReconciler_PathPattern(t *testing.T) { + g := NewWithT(t) + reconciler := getArtifactGeneratorReconciler() + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + // Create a namespace + ns, err := testEnv.CreateNamespace(ctx, "test-pattern") + g.Expect(err).ToNot(HaveOccurred()) + + // Create the ArtifactGenerator object + objKey := client.ObjectKey{ + Name: "test-pattern-gen", + Namespace: ns.Name, + } + obj := getArtifactGenerator(objKey) + obj.Spec.Sources = []swapi.SourceReference{ + { + Alias: fmt.Sprintf("%s-git", objKey.Name), + Kind: sourcev1.GitRepositoryKind, + Name: objKey.Name, + }, + } + obj.Spec.PathPattern = fmt.Sprintf("@%s-git/apps/{app}/envs/{env}", objKey.Name) + obj.Spec.OutputArtifacts = []swapi.OutputArtifact{ + { + Name: fmt.Sprintf("%s-{{ .app }}-{{ .env }}", objKey.Name), + Copy: []swapi.CopyOperation{ + { + From: fmt.Sprintf("@%s-git/apps/{{ .app }}/envs/{{ .env }}/**", objKey.Name), + To: "@artifact/", + }, + }, + }, + } + + err = testClient.Create(ctx, obj) + g.Expect(err).ToNot(HaveOccurred()) + + // Create the GitRepository source with multiple matching directories + gitFiles := []gotktestsrv.File{ + {Name: "apps/auth/envs/dev/app.yaml", Body: "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: auth-dev"}, + {Name: "apps/auth/envs/prod/app.yaml", Body: "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: auth-prod"}, + {Name: "apps/payments/envs/dev/app.yaml", Body: "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: payments-dev"}, + {Name: "apps/payments/envs/staging/app.yaml", Body: "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: payments-staging"}, + {Name: "apps/ignore-me/something/app.yaml", Body: "apiVersion: apps/v1"}, // Should not match pattern + } + err = applyGitRepository(objKey, "main@sha256:abc12345", gitFiles) + g.Expect(err).ToNot(HaveOccurred()) + + // Initialize the ArtifactGenerator with the finalizer + _, err = reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: objKey, + }) + g.Expect(err).ToNot(HaveOccurred()) + + // Reconcile to process the sources and build artifacts + _, err = reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: objKey, + }) + g.Expect(err).ToNot(HaveOccurred()) + + t.Run("generates external artifacts per matched directory", func(t *testing.T) { + g := NewWithT(t) + err = testClient.Get(ctx, objKey, obj) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(gotkconditions.IsReady(obj)).To(BeTrue()) + + // Verify that 4 ExternalArtifacts were created matching the 4 directories + expectedArtifacts := []struct { + name string + app string + env string + }{ + {name: fmt.Sprintf("%s-auth-dev", objKey.Name), app: "auth", env: "dev"}, + {name: fmt.Sprintf("%s-auth-prod", objKey.Name), app: "auth", env: "prod"}, + {name: fmt.Sprintf("%s-payments-dev", objKey.Name), app: "payments", env: "dev"}, + {name: fmt.Sprintf("%s-payments-staging", objKey.Name), app: "payments", env: "staging"}, + } + + g.Expect(obj.Status.Inventory).ToNot(BeNil()) + g.Expect(len(obj.Status.Inventory)).To(Equal(4)) + + fetcher := gotkfetch.New(gotkfetch.WithRetries(1), gotkfetch.WithUntar(gotktar.WithMaxUntarSize(gotktar.UnlimitedUntarSize))) + + for _, item := range expectedArtifacts { + ea := &sourcev1.ExternalArtifact{} + key := client.ObjectKey{Name: item.name, Namespace: obj.Namespace} + err = testClient.Get(ctx, key, ea) + g.Expect(err).ToNot(HaveOccurred()) + + // 1. Label assertion is strictly per-artifact + g.Expect(ea.Labels).To(HaveKeyWithValue("app", item.app)) + g.Expect(ea.Labels).To(HaveKeyWithValue("env", item.env)) + g.Expect(ea.Status.Artifact).ToNot(BeNil()) + + // 2. Artifact contents / Copy paths assertion + tmpDir, err := os.MkdirTemp("", "test-artifact-*") + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(tmpDir) + + err = fetcher.FetchWithContext(ctx, ea.Status.Artifact.URL, ea.Status.Artifact.Digest, tmpDir) + g.Expect(err).ToNot(HaveOccurred()) + + // The copy operation should have flattened the apps/{app}/envs/{env} into the root + // Verify that the correct app.yaml exists and contains the correct name + content, err := os.ReadFile(filepath.Join(tmpDir, "app.yaml")) + g.Expect(err).ToNot(HaveOccurred(), "app.yaml was not found in the root of the artifact") + g.Expect(string(content)).To(ContainSubstring(fmt.Sprintf("name: %s-%s", item.app, item.env))) + } + }) + + t.Run("removes external artifact when directory is deleted", func(t *testing.T) { + g := NewWithT(t) + + // Update Git repository to remove 'payments/envs/staging' + gitFiles = []gotktestsrv.File{ + {Name: "apps/auth/envs/dev/app.yaml", Body: "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: auth-dev"}, + {Name: "apps/auth/envs/prod/app.yaml", Body: "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: auth-prod"}, + {Name: "apps/payments/envs/dev/app.yaml", Body: "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: payments-dev"}, + } + err = applyGitRepository(objKey, "main@sha256:abc12346", gitFiles) + g.Expect(err).ToNot(HaveOccurred()) + + // Reconcile to process the update + _, err = reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: objKey, + }) + g.Expect(err).ToNot(HaveOccurred()) + + err = testClient.Get(ctx, objKey, obj) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(gotkconditions.IsReady(obj)).To(BeTrue()) + + // Should now only be 3 items in inventory + g.Expect(len(obj.Status.Inventory)).To(Equal(3)) + + // Verify the deleted one is gone + ea := &sourcev1.ExternalArtifact{} + key := client.ObjectKey{Name: fmt.Sprintf("%s-payments-staging", objKey.Name), Namespace: obj.Namespace} + err = testClient.Get(ctx, key, ea) + g.Expect(err).To(HaveOccurred()) // Should be NotFound + }) + + t.Run("updates revision selectively on content change", func(t *testing.T) { + g := NewWithT(t) + + // Record the current revision of auth-prod + eaProd := &sourcev1.ExternalArtifact{} + keyProd := client.ObjectKey{Name: fmt.Sprintf("%s-auth-prod", objKey.Name), Namespace: obj.Namespace} + err = testClient.Get(ctx, keyProd, eaProd) + g.Expect(err).ToNot(HaveOccurred()) + oldProdRevision := eaProd.Status.Artifact.Revision + + // Record the current revision of auth-dev + eaDev := &sourcev1.ExternalArtifact{} + keyDev := client.ObjectKey{Name: fmt.Sprintf("%s-auth-dev", objKey.Name), Namespace: obj.Namespace} + err = testClient.Get(ctx, keyDev, eaDev) + g.Expect(err).ToNot(HaveOccurred()) + oldDevRevision := eaDev.Status.Artifact.Revision + + // Change only the content of auth-dev + gitFiles = []gotktestsrv.File{ + {Name: "apps/auth/envs/dev/app.yaml", Body: "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: auth-dev-updated"}, + {Name: "apps/auth/envs/prod/app.yaml", Body: "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: auth-prod"}, + {Name: "apps/payments/envs/dev/app.yaml", Body: "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: payments-dev"}, + } + err = applyGitRepository(objKey, "main@sha256:abc12347", gitFiles) + g.Expect(err).ToNot(HaveOccurred()) + + // Reconcile + _, err = reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: objKey, + }) + g.Expect(err).ToNot(HaveOccurred()) + + // Verify auth-dev changed revision + err = testClient.Get(ctx, keyDev, eaDev) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(eaDev.Status.Artifact.Revision).ToNot(Equal(oldDevRevision)) + + // Verify auth-prod kept its original revision + err = testClient.Get(ctx, keyProd, eaProd) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(eaProd.Status.Artifact.Revision).To(Equal(oldProdRevision)) + }) +} diff --git a/internal/controller/artifactgenerator_drift_test.go b/internal/controller/artifactgenerator_drift_test.go index e2a2eeae..1d28e4ad 100644 --- a/internal/controller/artifactgenerator_drift_test.go +++ b/internal/controller/artifactgenerator_drift_test.go @@ -78,7 +78,7 @@ func TestArtifactGeneratorReconciler_DetectDrift(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "test-generator", Namespace: ns.Name, - }}, outputArtifact, artifact) + }}, outputArtifact, artifact, nil) g.Expect(err).ToNot(HaveOccurred()) tests := []struct { diff --git a/internal/controller/artifactgenerator_pathpattern.go b/internal/controller/artifactgenerator_pathpattern.go new file mode 100644 index 00000000..0d8c37af --- /dev/null +++ b/internal/controller/artifactgenerator_pathpattern.go @@ -0,0 +1,216 @@ +/* +Copyright 2025 The Flux authors + +Licensed 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. +*/ + +package controller + +import ( + "bytes" + "fmt" + "io/fs" + "path/filepath" + "regexp" + "strings" + "text/template" + + swapi "github.com/fluxcd/source-watcher/api/v2/v1beta1" + "k8s.io/apimachinery/pkg/api/validate/content" + apivalidation "k8s.io/apimachinery/pkg/api/validation" +) + +type artifactRequest struct { + swapi.OutputArtifact + Labels map[string]string +} + +// buildArtifactRequests expands the PathPattern into multiple artifact requests if specified. +// Otherwise, it returns the OutputArtifacts exactly as defined in the spec. +func buildArtifactRequests(obj *swapi.ArtifactGenerator, localSources map[string]string) ([]artifactRequest, error) { + if obj.Spec.PathPattern == "" { + reqs := make([]artifactRequest, 0, len(obj.Spec.OutputArtifacts)) + for _, oa := range obj.Spec.OutputArtifacts { + reqs = append(reqs, artifactRequest{ + OutputArtifact: oa, + Labels: nil, + }) + } + return reqs, nil + } + + // Parse @/ + parts := strings.SplitN(obj.Spec.PathPattern, "/", 2) + if len(parts) != 2 || !strings.HasPrefix(parts[0], "@") { + return nil, fmt.Errorf("invalid pathPattern format: %s", obj.Spec.PathPattern) + } + + alias := strings.TrimPrefix(parts[0], "@") + patternStr := parts[1] + + srcDir, ok := localSources[alias] + if !ok { + return nil, fmt.Errorf("source alias %s not found in local sources", alias) + } + + // Convert the path pattern (e.g. "apps/{app}/envs/{env}") to a regex + // with named capture groups (e.g. "^apps/(?P[^/]+)/envs/(?P[^/]+)$"). + escaped := regexp.QuoteMeta(patternStr) + captureRe := regexp.MustCompile(`\\\{([a-zA-Z0-9_]+)\\\}`) + regexStr := "^" + captureRe.ReplaceAllString(escaped, `(?P<$1>[^/]+)`) + "$" + matcher, err := regexp.Compile(regexStr) + if err != nil { + return nil, fmt.Errorf("failed to compile path pattern: %w", err) + } + + subexpNames := matcher.SubexpNames() + + var reqs []artifactRequest + // Track rendered artifact names to detect collisions after lowercasing. + seenNames := make(map[string]string) + + err = filepath.WalkDir(srcDir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if !d.IsDir() { + return nil + } + + rel, err := filepath.Rel(srcDir, path) + if err != nil { + return fmt.Errorf("failed to compute relative path for %s: %w", path, err) + } + if rel == "." { + return nil + } + // Normalize separators for cross-platform compatibility. + rel = filepath.ToSlash(rel) + + matches := matcher.FindStringSubmatch(rel) + if matches == nil { + return nil + } + + // Extract named captures from the regex match. + captures := make(map[string]string) + for i, name := range subexpNames { + if i != 0 && name != "" { + captures[name] = matches[i] + } + } + + // Run ToLower on all extracted values. + for k, v := range captures { + captures[k] = strings.ToLower(v) + } + + // Validate values with content.IsLabelValue. + for k, v := range captures { + if errs := content.IsLabelValue(v); len(errs) > 0 { + return fmt.Errorf( + "pathPattern %q: captured value %q for variable %q is not a valid Kubernetes label value: %s", + obj.Spec.PathPattern, v, k, strings.Join(errs, "; ")) + } + } + + // Render the OutputArtifacts using the captured variables. + for _, oa := range obj.Spec.OutputArtifacts { + req, err := renderArtifactRequest(oa, captures) + if err != nil { + return fmt.Errorf("failed to render artifact %s for match %s: %w", oa.Name, rel, err) + } + + // Validate rendered EA name with NameIsDNSSubdomain. + if errs := apivalidation.NameIsDNSSubdomain(req.Name, false); len(errs) > 0 { + return fmt.Errorf( + "pathPattern %q: rendered artifact name %q is not a valid Kubernetes object name: %s", + obj.Spec.PathPattern, req.Name, strings.Join(errs, "; ")) + } + + // Validate for duplicate names. + if prevDir, exists := seenNames[req.Name]; exists { + return fmt.Errorf( + "pathPattern %q: directories %q and %q both resolve to artifact name %q", + obj.Spec.PathPattern, prevDir, rel, req.Name) + } + seenNames[req.Name] = rel + + reqs = append(reqs, req) + } + + return nil + }) + + if err != nil { + return nil, fmt.Errorf("failed to walk source directory: %w", err) + } + + return reqs, nil +} + +// renderArtifactRequest creates an artifactRequest by evaluating Go template +// expressions in the OutputArtifact's Name and Copy fields using the provided captures. +func renderArtifactRequest(oa swapi.OutputArtifact, captures map[string]string) (artifactRequest, error) { + name, err := renderTemplate(oa.Name, captures) + if err != nil { + return artifactRequest{}, err + } + + req := artifactRequest{ + OutputArtifact: oa, + Labels: captures, + } + req.Name = name + + // Deep copy the Copy slice to avoid mutating the original OutputArtifact spec, + // and render template expressions in each copy operation. + req.Copy = make([]swapi.CopyOperation, len(oa.Copy)) + for i, copyOp := range oa.Copy { + from, err := renderTemplate(copyOp.From, captures) + if err != nil { + return artifactRequest{}, err + } + + to, err := renderTemplate(copyOp.To, captures) + if err != nil { + return artifactRequest{}, err + } + + req.Copy[i] = swapi.CopyOperation{ + From: from, + To: to, + Exclude: copyOp.Exclude, + Strategy: copyOp.Strategy, + } + } + + return req, nil +} + +// renderTemplate evaluates a Go template string with the provided data map. +// If the string does not contain any template delimiters, it is returned as-is. +func renderTemplate(tmplStr string, data map[string]string) (string, error) { + if !strings.Contains(tmplStr, "{{") { + return tmplStr, nil + } + tmpl, err := template.New("").Parse(tmplStr) + if err != nil { + return "", err + } + var buf bytes.Buffer + if err := tmpl.Execute(&buf, data); err != nil { + return "", err + } + return buf.String(), nil +} diff --git a/internal/controller/artifactgenerator_pathpattern_test.go b/internal/controller/artifactgenerator_pathpattern_test.go new file mode 100644 index 00000000..2b572b1f --- /dev/null +++ b/internal/controller/artifactgenerator_pathpattern_test.go @@ -0,0 +1,357 @@ +/* +Copyright 2025 The Flux authors + +Licensed 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. +*/ + +package controller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/onsi/gomega" + + swapi "github.com/fluxcd/source-watcher/api/v2/v1beta1" +) + +func TestBuildArtifactRequests(t *testing.T) { + g := gomega.NewWithT(t) + + tmpDir, err := os.MkdirTemp("", "pattern-test") + g.Expect(err).ToNot(gomega.HaveOccurred()) + defer os.RemoveAll(tmpDir) + + aliasDir := filepath.Join(tmpDir, "repo") + err = os.MkdirAll(aliasDir, 0o755) + g.Expect(err).ToNot(gomega.HaveOccurred()) + + localSources := map[string]string{ + "repo": aliasDir, + } + + createFile := func(path string) { + p := filepath.Join(aliasDir, path) + os.MkdirAll(filepath.Dir(p), 0o755) + os.WriteFile(p, []byte("test"), 0o644) + } + createDir := func(path string) { + os.MkdirAll(filepath.Join(aliasDir, path), 0o755) + } + + createDir("apps/auth/envs/dev") + createDir("apps/auth/envs/prod") + createDir("apps/payments") + createFile("apps/ignore/envs/dev") // this is a file, not a dir + + t.Run("static path (no pathPattern)", func(t *testing.T) { + g := gomega.NewWithT(t) + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + OutputArtifacts: []swapi.OutputArtifact{ + {Name: "static-1"}, + {Name: "static-2"}, + }, + }, + } + reqs, err := buildArtifactRequests(obj, localSources) + g.Expect(err).ToNot(gomega.HaveOccurred()) + g.Expect(reqs).To(gomega.HaveLen(2)) + g.Expect(reqs[0].Name).To(gomega.Equal("static-1")) + g.Expect(reqs[0].Labels).To(gomega.BeNil()) + }) + + t.Run("invalid pathPattern format", func(t *testing.T) { + g := gomega.NewWithT(t) + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "invalid-no-at-sign", + }, + } + _, err := buildArtifactRequests(obj, localSources) + g.Expect(err).To(gomega.HaveOccurred()) + g.Expect(err.Error()).To(gomega.ContainSubstring("invalid pathPattern format")) + }) + + t.Run("unknown alias in pattern", func(t *testing.T) { + g := gomega.NewWithT(t) + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@unknown/apps/{app}", + }, + } + _, err := buildArtifactRequests(obj, localSources) + g.Expect(err).To(gomega.HaveOccurred()) + g.Expect(err.Error()).To(gomega.ContainSubstring("not found in local sources")) + }) + + t.Run("single capture pattern", func(t *testing.T) { + g := gomega.NewWithT(t) + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@repo/apps/{app}", + OutputArtifacts: []swapi.OutputArtifact{ + { + Name: "app-{{ .app }}", + Copy: []swapi.CopyOperation{ + {From: "apps/{{ .app }}", To: "."}, + }, + }, + }, + }, + } + reqs, err := buildArtifactRequests(obj, localSources) + g.Expect(err).ToNot(gomega.HaveOccurred()) + g.Expect(reqs).To(gomega.HaveLen(3)) // auth, payments, and ignore + + names := []string{reqs[0].Name, reqs[1].Name, reqs[2].Name} + g.Expect(names).To(gomega.ConsistOf("app-auth", "app-payments", "app-ignore")) + + for _, r := range reqs { + if r.Name == "app-auth" { + g.Expect(r.Labels).To(gomega.HaveKeyWithValue("app", "auth")) + g.Expect(r.Copy[0].From).To(gomega.Equal("apps/auth")) + } + } + }) + + t.Run("preserves Exclude and Strategy fields", func(t *testing.T) { + g := gomega.NewWithT(t) + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@repo/apps/{app}", + OutputArtifacts: []swapi.OutputArtifact{ + { + Name: "app-{{ .app }}", + Copy: []swapi.CopyOperation{ + { + From: "apps/{{ .app }}", + To: ".", + Exclude: []string{"*.md", "tests/"}, + Strategy: "Merge", + }, + }, + }, + }, + }, + } + reqs, err := buildArtifactRequests(obj, localSources) + g.Expect(err).ToNot(gomega.HaveOccurred()) + g.Expect(reqs).To(gomega.HaveLen(3)) + + for _, r := range reqs { + g.Expect(r.Copy[0].Exclude).To(gomega.Equal([]string{"*.md", "tests/"})) + g.Expect(r.Copy[0].Strategy).To(gomega.Equal("Merge")) + } + }) + + t.Run("skips files that match pattern exactly", func(t *testing.T) { + g := gomega.NewWithT(t) + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@repo/apps/{app}/envs/{env}", + OutputArtifacts: []swapi.OutputArtifact{ + {Name: "{{ .app }}-{{ .env }}"}, + }, + }, + } + reqs, err := buildArtifactRequests(obj, localSources) + g.Expect(err).ToNot(gomega.HaveOccurred()) + g.Expect(reqs).To(gomega.HaveLen(2)) // only auth/envs/dev and auth/envs/prod, ignore is a file + + names := []string{reqs[0].Name, reqs[1].Name} + g.Expect(names).To(gomega.ConsistOf("auth-dev", "auth-prod")) + + for _, r := range reqs { + g.Expect(r.Labels["app"]).To(gomega.Equal("auth")) + g.Expect(r.Labels["env"]).To(gomega.BeElementOf("dev", "prod")) + } + }) + + t.Run("uppercase directories are lowercased", func(t *testing.T) { + g := gomega.NewWithT(t) + + upperDir, err := os.MkdirTemp("", "upper-test") + g.Expect(err).ToNot(gomega.HaveOccurred()) + defer os.RemoveAll(upperDir) + + upperAliasDir := filepath.Join(upperDir, "repo") + os.MkdirAll(filepath.Join(upperAliasDir, "apps", "Auth", "envs", "Dev"), 0o755) + os.MkdirAll(filepath.Join(upperAliasDir, "apps", "Payments", "envs", "Prod"), 0o755) + + upperSources := map[string]string{"repo": upperAliasDir} + + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@repo/apps/{app}/envs/{env}", + OutputArtifacts: []swapi.OutputArtifact{ + { + Name: "{{ .app }}-{{ .env }}", + Copy: []swapi.CopyOperation{ + {From: "@repo/apps/{{ .app }}/envs/{{ .env }}/**", To: "@artifact/"}, + }, + }, + }, + }, + } + reqs, err := buildArtifactRequests(obj, upperSources) + g.Expect(err).ToNot(gomega.HaveOccurred()) + g.Expect(reqs).To(gomega.HaveLen(2)) + + names := []string{reqs[0].Name, reqs[1].Name} + g.Expect(names).To(gomega.ConsistOf("auth-dev", "payments-prod")) + + for _, r := range reqs { + // Labels should contain lowercased values. + for _, v := range r.Labels { + g.Expect(v).To(gomega.Equal(strings.ToLower(v))) + } + } + }) + + t.Run("invalid label value - leading dot", func(t *testing.T) { + g := gomega.NewWithT(t) + + dotDir, err := os.MkdirTemp("", "dot-test") + g.Expect(err).ToNot(gomega.HaveOccurred()) + defer os.RemoveAll(dotDir) + + dotAliasDir := filepath.Join(dotDir, "repo") + os.MkdirAll(filepath.Join(dotAliasDir, "apps", ".hidden"), 0o755) + + dotSources := map[string]string{"repo": dotAliasDir} + + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@repo/apps/{app}", + OutputArtifacts: []swapi.OutputArtifact{ + {Name: "app-{{ .app }}"}, + }, + }, + } + _, err = buildArtifactRequests(obj, dotSources) + g.Expect(err).To(gomega.HaveOccurred()) + g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) + g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes label value")) + }) + + t.Run("invalid label value - contains space", func(t *testing.T) { + g := gomega.NewWithT(t) + + spaceDir, err := os.MkdirTemp("", "space-test") + g.Expect(err).ToNot(gomega.HaveOccurred()) + defer os.RemoveAll(spaceDir) + + spaceAliasDir := filepath.Join(spaceDir, "repo") + os.MkdirAll(filepath.Join(spaceAliasDir, "apps", "my app"), 0o755) + + spaceSources := map[string]string{"repo": spaceAliasDir} + + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@repo/apps/{app}", + OutputArtifacts: []swapi.OutputArtifact{ + {Name: "app-{{ .app }}"}, + }, + }, + } + _, err = buildArtifactRequests(obj, spaceSources) + g.Expect(err).To(gomega.HaveOccurred()) + g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) + g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes label value")) + }) + + t.Run("invalid label value - too long", func(t *testing.T) { + g := gomega.NewWithT(t) + + longDir, err := os.MkdirTemp("", "long-test") + g.Expect(err).ToNot(gomega.HaveOccurred()) + defer os.RemoveAll(longDir) + + longAliasDir := filepath.Join(longDir, "repo") + longName := strings.Repeat("a", 64) // 64 chars exceeds 63 char label value limit + os.MkdirAll(filepath.Join(longAliasDir, "apps", longName), 0o755) + + longSources := map[string]string{"repo": longAliasDir} + + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@repo/apps/{app}", + OutputArtifacts: []swapi.OutputArtifact{ + {Name: "app-{{ .app }}"}, + }, + }, + } + _, err = buildArtifactRequests(obj, longSources) + g.Expect(err).To(gomega.HaveOccurred()) + g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) + g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes label value")) + }) + + t.Run("invalid rendered DNS name", func(t *testing.T) { + g := gomega.NewWithT(t) + + dnsDir, err := os.MkdirTemp("", "dns-test") + g.Expect(err).ToNot(gomega.HaveOccurred()) + defer os.RemoveAll(dnsDir) + + dnsAliasDir := filepath.Join(dnsDir, "repo") + os.MkdirAll(filepath.Join(dnsAliasDir, "apps", "auth"), 0o755) + + dnsSources := map[string]string{"repo": dnsAliasDir} + + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@repo/apps/{app}", + OutputArtifacts: []swapi.OutputArtifact{ + // Template produces "auth..invalid" which is not a valid DNS subdomain. + {Name: "{{ .app }}..invalid"}, + }, + }, + } + _, err = buildArtifactRequests(obj, dnsSources) + g.Expect(err).To(gomega.HaveOccurred()) + g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) + g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes object name")) + }) + + t.Run("duplicate names after lowercasing", func(t *testing.T) { + g := gomega.NewWithT(t) + + dupDir, err := os.MkdirTemp("", "dup-test") + g.Expect(err).ToNot(gomega.HaveOccurred()) + defer os.RemoveAll(dupDir) + + dupAliasDir := filepath.Join(dupDir, "repo") + // Both will lowercase to "auth" + os.MkdirAll(filepath.Join(dupAliasDir, "apps", "Auth"), 0o755) + os.MkdirAll(filepath.Join(dupAliasDir, "apps", "auth"), 0o755) + + dupSources := map[string]string{"repo": dupAliasDir} + + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@repo/apps/{app}", + OutputArtifacts: []swapi.OutputArtifact{ + {Name: "app-{{ .app }}"}, + }, + }, + } + _, err = buildArtifactRequests(obj, dupSources) + g.Expect(err).To(gomega.HaveOccurred()) + g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) + g.Expect(err.Error()).To(gomega.ContainSubstring("both resolve to artifact name")) + }) +} From 55b62cfb4e0a3bfbe865a93a826461b44159d42e Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Wed, 24 Jun 2026 10:27:37 +0100 Subject: [PATCH 02/10] Remove drift detection heuristic for inventory size when pathPattern is used Signed-off-by: Matheus Pimenta --- .../controller/artifactgenerator_drift.go | 2 +- .../artifactgenerator_drift_test.go | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/internal/controller/artifactgenerator_drift.go b/internal/controller/artifactgenerator_drift.go index 9120e589..02cbbb74 100644 --- a/internal/controller/artifactgenerator_drift.go +++ b/internal/controller/artifactgenerator_drift.go @@ -68,7 +68,7 @@ func (r *ArtifactGeneratorReconciler) detectDrift(ctx context.Context, return true, "SourcesChanged" } - if len(obj.Status.Inventory) != len(obj.Spec.OutputArtifacts) { + if len(obj.Status.Inventory) != len(obj.Spec.OutputArtifacts) && obj.Spec.PathPattern == "" { log.Info("Drift detected, number of output artifacts has changed", "old", len(obj.Status.Inventory), "new", len(obj.Spec.OutputArtifacts)) diff --git a/internal/controller/artifactgenerator_drift_test.go b/internal/controller/artifactgenerator_drift_test.go index 1d28e4ad..5f7749a4 100644 --- a/internal/controller/artifactgenerator_drift_test.go +++ b/internal/controller/artifactgenerator_drift_test.go @@ -321,6 +321,43 @@ func TestArtifactGeneratorReconciler_DetectDrift(t *testing.T) { expectedDrift: true, expectedReason: "ExternalArtifactsChanged", }, + { + name: "no drift when pathPattern inventory count differs from templates", + obj: &swapi.ArtifactGenerator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "path-pattern-generator", + Namespace: ns.Name, + UID: "path-pattern-generator", + Generation: 1, + }, + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@test/apps/{app}/envs/{env}", + OutputArtifacts: []swapi.OutputArtifact{ + { + Name: "{{ .app }}-{{ .env }}", + Copy: []swapi.CopyOperation{ + {From: "@test/apps/{{ .app }}/envs/{{ .env }}/**", To: "@artifact/"}, + }, + }, + }, + }, + Status: swapi.ArtifactGeneratorStatus{ + Conditions: []metav1.Condition{ + { + Type: gotkmeta.ReadyCondition, + Status: metav1.ConditionTrue, + Reason: gotkmeta.SucceededReason, + ObservedGeneration: 1, + }, + }, + ObservedSourcesDigest: "test123", + Inventory: nil, + }, + }, + currentDigest: "test123", + expectedDrift: false, + expectedReason: "NoDriftDetected", + }, } for _, tt := range tests { From eb5333a69c9bc2543e4e62a331b6346aa570af9f Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Wed, 24 Jun 2026 10:55:10 +0100 Subject: [PATCH 03/10] Use raw captures when referring to the artifact file tree Signed-off-by: Matheus Pimenta --- .../artifactgenerator_pathpattern.go | 29 ++++++++++--------- .../artifactgenerator_pathpattern_test.go | 12 +++++++- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/internal/controller/artifactgenerator_pathpattern.go b/internal/controller/artifactgenerator_pathpattern.go index 0d8c37af..3852bf3e 100644 --- a/internal/controller/artifactgenerator_pathpattern.go +++ b/internal/controller/artifactgenerator_pathpattern.go @@ -103,20 +103,21 @@ func buildArtifactRequests(obj *swapi.ArtifactGenerator, localSources map[string } // Extract named captures from the regex match. - captures := make(map[string]string) + rawCaptures := make(map[string]string) for i, name := range subexpNames { if i != 0 && name != "" { - captures[name] = matches[i] + rawCaptures[name] = matches[i] } } - // Run ToLower on all extracted values. - for k, v := range captures { - captures[k] = strings.ToLower(v) + // Run ToLower on all extracted values used in Kubernetes metadata. + normalizedCaptures := make(map[string]string, len(rawCaptures)) + for k, v := range rawCaptures { + normalizedCaptures[k] = strings.ToLower(v) } // Validate values with content.IsLabelValue. - for k, v := range captures { + for k, v := range normalizedCaptures { if errs := content.IsLabelValue(v); len(errs) > 0 { return fmt.Errorf( "pathPattern %q: captured value %q for variable %q is not a valid Kubernetes label value: %s", @@ -126,7 +127,7 @@ func buildArtifactRequests(obj *swapi.ArtifactGenerator, localSources map[string // Render the OutputArtifacts using the captured variables. for _, oa := range obj.Spec.OutputArtifacts { - req, err := renderArtifactRequest(oa, captures) + req, err := renderArtifactRequest(oa, normalizedCaptures, rawCaptures) if err != nil { return fmt.Errorf("failed to render artifact %s for match %s: %w", oa.Name, rel, err) } @@ -159,17 +160,17 @@ func buildArtifactRequests(obj *swapi.ArtifactGenerator, localSources map[string return reqs, nil } -// renderArtifactRequest creates an artifactRequest by evaluating Go template -// expressions in the OutputArtifact's Name and Copy fields using the provided captures. -func renderArtifactRequest(oa swapi.OutputArtifact, captures map[string]string) (artifactRequest, error) { - name, err := renderTemplate(oa.Name, captures) +// renderArtifactRequest creates an artifactRequest by evaluating Go template expressions. +// Artifact names and labels use normalized captures, while copy paths use raw captures. +func renderArtifactRequest(oa swapi.OutputArtifact, normalizedCaptures, rawCaptures map[string]string) (artifactRequest, error) { + name, err := renderTemplate(oa.Name, normalizedCaptures) if err != nil { return artifactRequest{}, err } req := artifactRequest{ OutputArtifact: oa, - Labels: captures, + Labels: normalizedCaptures, } req.Name = name @@ -177,12 +178,12 @@ func renderArtifactRequest(oa swapi.OutputArtifact, captures map[string]string) // and render template expressions in each copy operation. req.Copy = make([]swapi.CopyOperation, len(oa.Copy)) for i, copyOp := range oa.Copy { - from, err := renderTemplate(copyOp.From, captures) + from, err := renderTemplate(copyOp.From, rawCaptures) if err != nil { return artifactRequest{}, err } - to, err := renderTemplate(copyOp.To, captures) + to, err := renderTemplate(copyOp.To, rawCaptures) if err != nil { return artifactRequest{}, err } diff --git a/internal/controller/artifactgenerator_pathpattern_test.go b/internal/controller/artifactgenerator_pathpattern_test.go index 2b572b1f..86c441ac 100644 --- a/internal/controller/artifactgenerator_pathpattern_test.go +++ b/internal/controller/artifactgenerator_pathpattern_test.go @@ -200,7 +200,7 @@ func TestBuildArtifactRequests(t *testing.T) { { Name: "{{ .app }}-{{ .env }}", Copy: []swapi.CopyOperation{ - {From: "@repo/apps/{{ .app }}/envs/{{ .env }}/**", To: "@artifact/"}, + {From: "@repo/apps/{{ .app }}/envs/{{ .env }}/**", To: "@artifact/{{ .app }}/{{ .env }}/"}, }, }, }, @@ -218,6 +218,16 @@ func TestBuildArtifactRequests(t *testing.T) { for _, v := range r.Labels { g.Expect(v).To(gomega.Equal(strings.ToLower(v))) } + + switch r.Name { + case "auth-dev": + g.Expect(r.Copy[0].From).To(gomega.Equal("@repo/apps/Auth/envs/Dev/**")) + g.Expect(r.Copy[0].To).To(gomega.Equal("@artifact/Auth/Dev/")) + case "payments-prod": + g.Expect(r.Copy[0].From).To(gomega.Equal("@repo/apps/Payments/envs/Prod/**")) + g.Expect(r.Copy[0].To).To(gomega.Equal("@artifact/Payments/Prod/")) + } + } }) From e88b4253a8b8b8c65e117d73ffbaddc71a12114b Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Wed, 24 Jun 2026 11:01:21 +0100 Subject: [PATCH 04/10] Validate pathPattern capture names as Kubernetes label keys Signed-off-by: Matheus Pimenta --- .../controller/artifactgenerator_pathpattern.go | 7 +++++++ .../artifactgenerator_pathpattern_test.go | 14 ++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/internal/controller/artifactgenerator_pathpattern.go b/internal/controller/artifactgenerator_pathpattern.go index 3852bf3e..e39c3720 100644 --- a/internal/controller/artifactgenerator_pathpattern.go +++ b/internal/controller/artifactgenerator_pathpattern.go @@ -74,6 +74,13 @@ func buildArtifactRequests(obj *swapi.ArtifactGenerator, localSources map[string } subexpNames := matcher.SubexpNames() + for _, name := range subexpNames[1:] { + if errs := content.IsLabelKey(name); len(errs) > 0 { + return nil, fmt.Errorf( + "pathPattern %q: capture variable %q is not a valid Kubernetes label key: %s", + obj.Spec.PathPattern, name, strings.Join(errs, "; ")) + } + } var reqs []artifactRequest // Track rendered artifact names to detect collisions after lowercasing. diff --git a/internal/controller/artifactgenerator_pathpattern_test.go b/internal/controller/artifactgenerator_pathpattern_test.go index 86c441ac..b95dee50 100644 --- a/internal/controller/artifactgenerator_pathpattern_test.go +++ b/internal/controller/artifactgenerator_pathpattern_test.go @@ -97,6 +97,20 @@ func TestBuildArtifactRequests(t *testing.T) { g.Expect(err.Error()).To(gomega.ContainSubstring("not found in local sources")) }) + t.Run("invalid capture label key", func(t *testing.T) { + g := gomega.NewWithT(t) + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@repo/apps/{_app}", + }, + } + _, err := buildArtifactRequests(obj, localSources) + g.Expect(err).To(gomega.HaveOccurred()) + g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) + g.Expect(err.Error()).To(gomega.ContainSubstring("capture variable")) + g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes label key")) + }) + t.Run("single capture pattern", func(t *testing.T) { g := gomega.NewWithT(t) obj := &swapi.ArtifactGenerator{ From 3eb5445161ecbe2ec99814d4fafa533d60a7237a Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Wed, 24 Jun 2026 11:20:19 +0100 Subject: [PATCH 05/10] Sanitize pathPattern captures Signed-off-by: Matheus Pimenta --- .../artifactgenerator_pathpattern.go | 35 +++++++++++++++++++ .../artifactgenerator_pathpattern_test.go | 30 ++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/internal/controller/artifactgenerator_pathpattern.go b/internal/controller/artifactgenerator_pathpattern.go index e39c3720..336ff7fe 100644 --- a/internal/controller/artifactgenerator_pathpattern.go +++ b/internal/controller/artifactgenerator_pathpattern.go @@ -35,6 +35,8 @@ type artifactRequest struct { Labels map[string]string } +var pathPatternCaptureNameRe = regexp.MustCompile("^[A-Za-z0-9_]+$") + // buildArtifactRequests expands the PathPattern into multiple artifact requests if specified. // Otherwise, it returns the OutputArtifacts exactly as defined in the spec. func buildArtifactRequests(obj *swapi.ArtifactGenerator, localSources map[string]string) ([]artifactRequest, error) { @@ -66,6 +68,10 @@ func buildArtifactRequests(obj *swapi.ArtifactGenerator, localSources map[string // Convert the path pattern (e.g. "apps/{app}/envs/{env}") to a regex // with named capture groups (e.g. "^apps/(?P[^/]+)/envs/(?P[^/]+)$"). escaped := regexp.QuoteMeta(patternStr) + if err := validatePathPatternCaptures(obj.Spec.PathPattern, patternStr); err != nil { + return nil, err + } + captureRe := regexp.MustCompile(`\\\{([a-zA-Z0-9_]+)\\\}`) regexStr := "^" + captureRe.ReplaceAllString(escaped, `(?P<$1>[^/]+)`) + "$" matcher, err := regexp.Compile(regexStr) @@ -167,6 +173,35 @@ func buildArtifactRequests(obj *swapi.ArtifactGenerator, localSources map[string return reqs, nil } +func validatePathPatternCaptures(pathPattern, patternStr string) error { + seen := make(map[string]struct{}) + for i := 0; i < len(patternStr); i++ { + switch patternStr[i] { + case '{': + end := strings.IndexByte(patternStr[i+1:], '}') + if end < 0 { + return fmt.Errorf("pathPattern %q: invalid capture starting at offset %d: missing closing brace", pathPattern, i) + } + + name := patternStr[i+1 : i+1+end] + if name == "" { + return fmt.Errorf("pathPattern %q: empty capture variable", pathPattern) + } + if !pathPatternCaptureNameRe.MatchString(name) { + return fmt.Errorf("pathPattern %q: capture variable %q must match [A-Za-z0-9_]+", pathPattern, name) + } + if _, ok := seen[name]; ok { + return fmt.Errorf("pathPattern %q: duplicate capture variable %q", pathPattern, name) + } + seen[name] = struct{}{} + i += end + 1 + case '}': + return fmt.Errorf("pathPattern %q: invalid capture ending at offset %d: missing opening brace", pathPattern, i) + } + } + return nil +} + // renderArtifactRequest creates an artifactRequest by evaluating Go template expressions. // Artifact names and labels use normalized captures, while copy paths use raw captures. func renderArtifactRequest(oa swapi.OutputArtifact, normalizedCaptures, rawCaptures map[string]string) (artifactRequest, error) { diff --git a/internal/controller/artifactgenerator_pathpattern_test.go b/internal/controller/artifactgenerator_pathpattern_test.go index b95dee50..4a303c44 100644 --- a/internal/controller/artifactgenerator_pathpattern_test.go +++ b/internal/controller/artifactgenerator_pathpattern_test.go @@ -111,6 +111,36 @@ func TestBuildArtifactRequests(t *testing.T) { g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes label key")) }) + t.Run("invalid capture syntax", func(t *testing.T) { + cases := []struct { + name string + pattern string + wantErrMsg string + }{ + {name: "empty capture", pattern: "@repo/apps/{}", wantErrMsg: "empty capture variable"}, + {name: "unsupported capture name", pattern: "@repo/apps/{app-name}", wantErrMsg: "must match [A-Za-z0-9_]+"}, + {name: "missing closing brace", pattern: "@repo/apps/{app", wantErrMsg: "missing closing brace"}, + {name: "missing opening brace", pattern: "@repo/apps/app}", wantErrMsg: "missing opening brace"}, + {name: "duplicate capture", pattern: "@repo/apps/{app}/envs/{app}", wantErrMsg: "duplicate capture variable"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := gomega.NewWithT(t) + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: tc.pattern, + }, + } + _, err := buildArtifactRequests(obj, localSources) + g.Expect(err).To(gomega.HaveOccurred()) + g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) + g.Expect(err.Error()).To(gomega.ContainSubstring(tc.pattern)) + g.Expect(err.Error()).To(gomega.ContainSubstring(tc.wantErrMsg)) + }) + } + }) + t.Run("single capture pattern", func(t *testing.T) { g := gomega.NewWithT(t) obj := &swapi.ArtifactGenerator{ From 3e23678761a20120220051e4fc7b8250e9951a7f Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Wed, 24 Jun 2026 11:49:50 +0100 Subject: [PATCH 06/10] Improve artifact name validation when pathPattern is not used Signed-off-by: Matheus Pimenta --- api/v1beta1/artifactgenerator_types.go | 1 + ...tensions.fluxcd.io_artifactgenerators.yaml | 5 ++++ .../artifactgenerator_validation.go | 11 +++++++ .../artifactgenerator_validation_test.go | 29 +++++++++++++++++++ 4 files changed, 46 insertions(+) diff --git a/api/v1beta1/artifactgenerator_types.go b/api/v1beta1/artifactgenerator_types.go index dd648a87..3a94cb89 100644 --- a/api/v1beta1/artifactgenerator_types.go +++ b/api/v1beta1/artifactgenerator_types.go @@ -54,6 +54,7 @@ type CommonMetadata struct { } // ArtifactGeneratorSpec defines the desired state of ArtifactGenerator. +// +kubebuilder:validation:XValidation:rule="has(self.pathPattern) && self.pathPattern != '' || self.artifacts.all(a, a.name.matches('^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$'))",message="artifact names must be valid Kubernetes object names when pathPattern is not set" type ArtifactGeneratorSpec struct { // CommonMetadata specifies the common labels and annotations that are // applied to all resources. Any existing label or annotation will be diff --git a/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml b/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml index 3738f93e..da45bccb 100644 --- a/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml +++ b/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml @@ -218,6 +218,11 @@ spec: - artifacts - sources type: object + x-kubernetes-validations: + - message: artifact names must be valid Kubernetes object names when pathPattern + is not set + rule: has(self.pathPattern) && self.pathPattern != '' || self.artifacts.all(a, + a.name.matches('^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$')) status: description: ArtifactGeneratorStatus defines the observed state of ArtifactGenerator. properties: diff --git a/internal/controller/artifactgenerator_validation.go b/internal/controller/artifactgenerator_validation.go index eb553715..76e020dd 100644 --- a/internal/controller/artifactgenerator_validation.go +++ b/internal/controller/artifactgenerator_validation.go @@ -19,6 +19,8 @@ package controller import ( "strings" + apivalidation "k8s.io/apimachinery/pkg/api/validation" + swapi "github.com/fluxcd/source-watcher/api/v2/v1beta1" ) @@ -54,6 +56,15 @@ func (r *ArtifactGeneratorReconciler) validateSpec(obj *swapi.ArtifactGenerator) "duplicate artifact name '%s' found", artifact.Name) } + if obj.Spec.PathPattern == "" { + if errs := apivalidation.NameIsDNSSubdomain(artifact.Name, false); len(errs) > 0 { + return r.newTerminalErrorFor(obj, + swapi.ValidationFailedReason, + "artifact name %q is not a valid Kubernetes object name: %s", + artifact.Name, strings.Join(errs, "; ")) + } + } + // Check that the revision source alias exists. if artifact.Revision != "" && !aliasMap[strings.TrimPrefix(artifact.Revision, "@")] { return r.newTerminalErrorFor(obj, diff --git a/internal/controller/artifactgenerator_validation_test.go b/internal/controller/artifactgenerator_validation_test.go index 927cb438..751e4ad7 100644 --- a/internal/controller/artifactgenerator_validation_test.go +++ b/internal/controller/artifactgenerator_validation_test.go @@ -234,6 +234,35 @@ func TestArtifactGenerator_crdValidation(t *testing.T) { }, expectError: false, }, + { + name: "invalid static artifact name", + setupObj: func() *swapi.ArtifactGenerator { + objKey := client.ObjectKey{ + Name: "test-invalid-static-artifact-name", + Namespace: ns.Name, + } + obj := getArtifactGenerator(objKey) + obj.Spec.OutputArtifacts[0].Name = "Invalid Name" + return obj + }, + expectError: true, + }, + { + name: "templated artifact name with pathPattern", + setupObj: func() *swapi.ArtifactGenerator { + objKey := client.ObjectKey{ + Name: "test-path-pattern-template-name", + Namespace: ns.Name, + } + obj := getArtifactGenerator(objKey) + alias := obj.Spec.Sources[0].Alias + obj.Spec.PathPattern = "@" + alias + "/apps/{app}" + obj.Spec.OutputArtifacts[0].Name = "{{ .app }}" + obj.Spec.OutputArtifacts[0].Copy[0].From = "@" + alias + "/apps/{{ .app }}/**" + return obj + }, + expectError: false, + }, { name: "invalid artifact copy from", setupObj: func() *swapi.ArtifactGenerator { From 884f847462eb7505a61f665aa576d8c3bba2a937 Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Wed, 24 Jun 2026 12:18:32 +0100 Subject: [PATCH 07/10] Change pathPattern Go templates to CEL expressions Signed-off-by: Matheus Pimenta --- api/v1beta1/artifactgenerator_types.go | 19 +++-- ...tensions.fluxcd.io_artifactgenerators.yaml | 22 ++++-- docs/spec/v1beta1/artifactgenerators.md | 22 +++--- go.mod | 8 ++ go.sum | 17 ++++ .../artifactgenerator_controller.go | 2 +- .../artifactgenerator_controller_test.go | 6 +- .../artifactgenerator_drift_test.go | 4 +- .../artifactgenerator_pathpattern.go | 57 ++++++++------ .../artifactgenerator_pathpattern_test.go | 77 ++++++++++++------- .../artifactgenerator_validation_test.go | 7 +- 11 files changed, 156 insertions(+), 85 deletions(-) diff --git a/api/v1beta1/artifactgenerator_types.go b/api/v1beta1/artifactgenerator_types.go index 3a94cb89..5554a0c4 100644 --- a/api/v1beta1/artifactgenerator_types.go +++ b/api/v1beta1/artifactgenerator_types.go @@ -54,7 +54,7 @@ type CommonMetadata struct { } // ArtifactGeneratorSpec defines the desired state of ArtifactGenerator. -// +kubebuilder:validation:XValidation:rule="has(self.pathPattern) && self.pathPattern != '' || self.artifacts.all(a, a.name.matches('^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$'))",message="artifact names must be valid Kubernetes object names when pathPattern is not set" +// +kubebuilder:validation:XValidation:rule="has(self.pathPattern) && size(self.pathPattern) > 0 || self.artifacts.all(a, a.name.matches('^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$'))",message="artifact names must be valid Kubernetes object names when pathPattern is not set" type ArtifactGeneratorSpec struct { // CommonMetadata specifies the common labels and annotations that are // applied to all resources. Any existing label or annotation will be @@ -71,7 +71,7 @@ type ArtifactGeneratorSpec struct { // PathPattern specifies a directory traversal pattern to match within the sources. // The format is "@/". Named captures in the pattern (e.g. "{app}") - // can be used as Go template variables in OutputArtifacts fields. + // are injected as string variables into OutputArtifacts CEL expressions. // +kubebuilder:validation:Pattern="^@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)$" // +kubebuilder:validation:MaxLength=1024 // +optional @@ -119,7 +119,8 @@ type SourceReference struct { // generated by the ArtifactGenerator. type OutputArtifact struct { // Name is the name of the generated artifact. - // +kubebuilder:validation:Pattern="^[a-zA-Z0-9{]([a-zA-Z0-9\\-\\{\\}\\.\\s]*[a-zA-Z0-9}])?$" + // When pathPattern is set, this field is evaluated as a CEL string expression. + // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=253 // +required Name string `json:"name"` @@ -153,16 +154,20 @@ type OutputArtifact struct { type CopyOperation struct { // From specifies the source (by alias) and the glob pattern to match files. - // The format is "@/". - // +kubebuilder:validation:Pattern="^@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)$" + // The format is "@/". When pathPattern is set, + // this field is evaluated as a CEL string expression. + // +kubebuilder:validation:Pattern="^(@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)|.*['\"+].*)$" + // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=1024 // +required From string `json:"from"` // To specifies the destination path within the artifact. // The format is "@artifact/path", the alias "artifact" - // refers to the root path of the generated artifact. - // +kubebuilder:validation:Pattern="^@(artifact)/(.*)$" + // refers to the root path of the generated artifact. When pathPattern + // is set, this field is evaluated as a CEL string expression. + // +kubebuilder:validation:Pattern="^(@(artifact)/(.*)|.*['\"+].*)$" + // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=1024 // +required To string `json:"to"` diff --git a/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml b/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml index da45bccb..e231f830 100644 --- a/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml +++ b/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml @@ -79,9 +79,11 @@ spec: from: description: |- From specifies the source (by alias) and the glob pattern to match files. - The format is "@/". + The format is "@/". When pathPattern is set, + this field is evaluated as a CEL string expression. maxLength: 1024 - pattern: ^@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)$ + minLength: 1 + pattern: ^(@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)|.*['"+].*)$ type: string strategy: description: |- @@ -101,9 +103,11 @@ spec: description: |- To specifies the destination path within the artifact. The format is "@artifact/path", the alias "artifact" - refers to the root path of the generated artifact. + refers to the root path of the generated artifact. When pathPattern + is set, this field is evaluated as a CEL string expression. maxLength: 1024 - pattern: ^@(artifact)/(.*)$ + minLength: 1 + pattern: ^(@(artifact)/(.*)|.*['"+].*)$ type: string required: - from @@ -112,9 +116,11 @@ spec: minItems: 1 type: array name: - description: Name is the name of the generated artifact. + description: |- + Name is the name of the generated artifact. + When pathPattern is set, this field is evaluated as a CEL string expression. maxLength: 253 - pattern: ^[a-zA-Z0-9{]([a-zA-Z0-9\-\{\}\.\s]*[a-zA-Z0-9}])?$ + minLength: 1 type: string originRevision: description: |- @@ -163,7 +169,7 @@ spec: description: |- PathPattern specifies a directory traversal pattern to match within the sources. The format is "@/". Named captures in the pattern (e.g. "{app}") - can be used as Go template variables in OutputArtifacts fields. + are injected as string variables into OutputArtifacts CEL expressions. maxLength: 1024 pattern: ^@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)$ type: string @@ -221,7 +227,7 @@ spec: x-kubernetes-validations: - message: artifact names must be valid Kubernetes object names when pathPattern is not set - rule: has(self.pathPattern) && self.pathPattern != '' || self.artifacts.all(a, + rule: has(self.pathPattern) && size(self.pathPattern) > 0 || self.artifacts.all(a, a.name.matches('^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$')) status: description: ArtifactGeneratorStatus defines the observed state of ArtifactGenerator. diff --git a/docs/spec/v1beta1/artifactgenerators.md b/docs/spec/v1beta1/artifactgenerators.md index 6522a579..90cf033a 100644 --- a/docs/spec/v1beta1/artifactgenerators.md +++ b/docs/spec/v1beta1/artifactgenerators.md @@ -232,7 +232,8 @@ regenerate the affected artifacts automatically. The `.spec.pathPattern` field allows for dynamic, path-based discovery of artifacts. When specified, the controller will scan the referenced source for directories matching the pattern and dynamically generate one ExternalArtifact for each matched directory. - `pathPattern` (optional): Specifies a directory traversal pattern within a source in the format `@/`. - Named captures in the pattern (e.g., `{app}`, `{env}`) can be used as Go template variables (`{{ .app }}`, `{{ .env }}`) inside the `artifacts` fields. + Named captures in the pattern (e.g., `{app}`, `{env}`) are injected as string variables into CEL expressions in the `artifacts` fields. + The `pathPattern` itself is not a CEL expression. When `pathPattern` is used, the generated ExternalArtifacts will automatically have their labels populated with the extracted capture variables. @@ -244,10 +245,10 @@ spec: name: my-monorepo pathPattern: "@monorepo/apps/{app}/envs/{env}" artifacts: - - name: "{{ .app }}-{{ .env }}" + - name: "app + '-' + env" copy: - - from: "@monorepo/apps/{{ .app }}/envs/{{ .env }}/**" - to: "@artifact/" + - from: "'@monorepo/apps/' + app + '/envs/' + env + '/**'" + to: "'@artifact/'" ``` #### Directory Name Constraints @@ -259,9 +260,10 @@ Directory names matched by path pattern captures must comply with - Must begin and end with an alphanumeric character (`[a-zA-Z0-9]`) - May contain dashes (`-`), underscores (`_`), dots (`.`), and alphanumeric characters -The controller automatically lowercases all captured values before using them +The controller automatically lowercases captured values before using them in artifact names and labels. This means a directory named `Auth` will produce -an artifact with `app=auth`. +an artifact with `app=auth`. Copy expressions receive the original captured +path values so source paths preserve their case. **Note:** If two directories differ only in case (e.g., `apps/Auth/` and `apps/auth/`), they will resolve to the same artifact name after lowercasing. In this case, the @@ -275,11 +277,11 @@ the `pathPattern` and the invalid value. ### Artifacts The `.spec.artifacts` field defines the list of ExternalArtifacts to be generated from the sources. -When `pathPattern` is defined, the artifacts act as a template for each matched directory. +When `pathPattern` is defined, the artifacts act as expression templates for each matched directory. Each artifact must specify: - `name` (required): The name of the generated ExternalArtifact resource. It must be unique in the context - of the ArtifactGenerator and must conform to Kubernetes resource naming conventions. Supports Go template variables if `pathPattern` is used. + of the ArtifactGenerator and must conform to Kubernetes resource naming conventions. Supports CEL expressions if `pathPattern` is used. - `copy` (required): A list of copy operations to perform from sources to the artifact. - `revision` (optional): A specific source revision to use in the format `@alias`. If not specified, the revision is automatically computed as `latest@` based on the artifact content. @@ -308,10 +310,12 @@ spec: Each copy operation specifies how to copy files from sources into the generated artifact: -- `from`: Source path in the format `@alias/pattern` where `alias` references +- `from`: Source path in the format `@alias/pattern` where `alias` references a source and `pattern` is a glob pattern or a specific file/directory path within that source. + When `pathPattern` is set, this field is a CEL string expression that must evaluate to this format. - `to`: Destination path in the format `@artifact/path` where `artifact` is the root of the generated artifact and `path` is the relative path to a file or directory. + When `pathPattern` is set, this field is a CEL string expression that must evaluate to this format. - `exclude` (optional): A list of glob patterns to filter out from the source selection. Any file matched by `from` that also matches an exclude pattern will be ignored. Patterns are matched against paths relative to the source alias root or to the diff --git a/go.mod b/go.mod index 8f5365c1..1f8d65a7 100644 --- a/go.mod +++ b/go.mod @@ -32,9 +32,11 @@ require ( ) require ( + cel.dev/expr v0.25.1 // indirect github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect github.com/MakeNowJust/heredoc v1.0.0 // indirect github.com/Masterminds/semver/v3 v3.5.0 // indirect + github.com/antlr4-go/antlr/v4 v4.13.0 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect @@ -48,6 +50,7 @@ require ( github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f // indirect github.com/fluxcd/cli-utils v1.2.1 // indirect github.com/fluxcd/pkg/apis/acl v0.10.0 // indirect + github.com/fluxcd/pkg/apis/kustomize v1.19.0 // indirect github.com/fluxcd/pkg/lockedfile v0.8.0 // indirect github.com/fluxcd/pkg/oci v0.68.0 // indirect github.com/fluxcd/pkg/sourceignore v0.18.0 // indirect @@ -72,6 +75,7 @@ require ( github.com/go-openapi/swag/typeutils v0.25.4 // indirect github.com/go-openapi/swag/yamlutils v0.25.4 // indirect github.com/google/btree v1.1.3 // indirect + github.com/google/cel-go v0.26.1 // indirect github.com/google/gnostic-models v0.7.0 // indirect github.com/google/go-cmp v0.7.0 // indirect github.com/google/go-containerregistry v0.21.6 // indirect @@ -101,6 +105,7 @@ require ( github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/sirupsen/logrus v1.9.4 // indirect github.com/spf13/cobra v1.10.2 // indirect + github.com/stoewer/go-strcase v1.3.0 // indirect github.com/x448/float16 v0.8.4 // indirect github.com/xlab/treeprint v1.2.0 // indirect github.com/zeebo/blake3 v0.2.4 // indirect @@ -108,6 +113,7 @@ require ( go.uber.org/zap v1.27.1 // indirect go.yaml.in/yaml/v2 v2.4.4 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect + golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93 // indirect golang.org/x/net v0.55.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sync v0.20.0 // indirect @@ -116,6 +122,8 @@ require ( golang.org/x/text v0.37.0 // indirect golang.org/x/time v0.15.0 // indirect gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9 // indirect google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af // indirect gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect diff --git a/go.sum b/go.sum index 5e13839c..7dc6fcb9 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +cel.dev/expr v0.25.1 h1:1KrZg61W6TWSxuNZ37Xy49ps13NUovb66QLprthtwi4= +cel.dev/expr v0.25.1/go.mod h1:hrXvqGP6G6gyx8UAHSHJ5RGk//1Oj5nXQ2NI02Nrsg4= github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6 h1:He8afgbRMd7mFxO99hRNu+6tazq8nFF9lIwo9JFroBk= github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6/go.mod h1:8o94RPi1/7XTJvwPpRSzSUedZrtlirdB3r9Z20bi2f8= github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 h1:L/gRVlceqvL25UVaW/CKtUDjefjrs0SPonmDGUVOYP0= @@ -6,6 +8,8 @@ github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE= github.com/Masterminds/semver/v3 v3.5.0 h1:kQceYJfbupGfZOKZQg0kou0DgAKhzDg2NZPAwZ/2OOE= github.com/Masterminds/semver/v3 v3.5.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= +github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI= +github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= @@ -63,6 +67,8 @@ github.com/fluxcd/pkg/apis/acl v0.10.0 h1:KPfAmELNvtvaz8wixnm/MYXqa+MJf7ntVVMUU9 github.com/fluxcd/pkg/apis/acl v0.10.0/go.mod h1:a87i2A7AlFO5N2J8CxtzaUCCDmuLLWOHwkKu3eJF5fY= github.com/fluxcd/pkg/apis/event v0.27.0 h1:fUJRyWU3sEKjV6SzBnoJT6aDP5cJdMAbspZyRfhde6I= github.com/fluxcd/pkg/apis/event v0.27.0/go.mod h1:0zua8dB0E9SXryScEf7LDMuUYVyVKgA/Xeh2h2gtSRU= +github.com/fluxcd/pkg/apis/kustomize v1.19.0 h1:DKiRUwS/8HaePXUF+JUShbBe/HAClnJwm2tdiUnPMiY= +github.com/fluxcd/pkg/apis/kustomize v1.19.0/go.mod h1:BYPKRhf3MQKPtSsaNcOs44BigF0q8Fzza5iuOHTd26U= github.com/fluxcd/pkg/apis/meta v1.30.0 h1:26TOd1hbamH3c5KOb/CIMGpUDB4G4JV+WCcPyUhmuaM= github.com/fluxcd/pkg/apis/meta v1.30.0/go.mod h1:q1YjUeCmf0syhkZoMcRmP3pkBaLKFLl7g0mUvVGG4CM= github.com/fluxcd/pkg/artifact v0.18.0 h1:TrIcQ6TaQ+787IA3YESFnihPqbfptqHHyqHZ2y0Cnsk= @@ -135,6 +141,8 @@ github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1v github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= github.com/google/btree v1.1.3 h1:CVpQJjYgC4VbzxeGVHfvZrv1ctoYCAI8vbl07Fcxlyg= github.com/google/btree v1.1.3/go.mod h1:qOPhT0dTNdNzV6Z/lhRX0YXUafgPLFUh+gZMl761Gm4= +github.com/google/cel-go v0.26.1 h1:iPbVVEdkhTX++hpe3lzSk7D3G3QSYqLGoHOcEio+UXQ= +github.com/google/cel-go v0.26.1/go.mod h1:A9O8OU9rdvrK5MQyrqfIxo1a0u4g3sF8KB6PUIaryMM= github.com/google/gnostic-models v0.7.0 h1:qwTtogB15McXDaNqTZdzPJRHvaVJlAl+HVQnLmJEJxo= github.com/google/gnostic-models v0.7.0/go.mod h1:whL5G0m6dmc5cPxKc5bdKdEN3UjI7OUGxBlw57miDrQ= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= @@ -244,12 +252,19 @@ github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiT github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/stoewer/go-strcase v1.3.0 h1:g0eASXYtp+yvN9fK8sH94oCIk0fau9uV1/ZdJ0AVEzs= +github.com/stoewer/go-strcase v1.3.0/go.mod h1:fAH5hQ5pehh+j3nZfvwdk2RgEgQjAoM8wodgtPmh1xo= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= @@ -320,6 +335,8 @@ go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI= golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8= +golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93 h1:fQsdNF2N+/YewlRZiricy4P1iimyPKZ/xwniHj8Q2a0= +golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93/go.mod h1:EPRbTFwzwjXj9NpYyyrvenVh9Y+GFeEvMNh7Xuz7xgU= golang.org/x/mod v0.36.0 h1:JJjpVx6myfUsUdAzZuOSTTmRE0PfZeNWzzvKrP7amb4= golang.org/x/mod v0.36.0/go.mod h1:moc6ELqsWcOw5Ef3xVprK5ul/MvtVvkIXLziUOICjUQ= golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= diff --git a/internal/controller/artifactgenerator_controller.go b/internal/controller/artifactgenerator_controller.go index 79d67b62..61ea0989 100644 --- a/internal/controller/artifactgenerator_controller.go +++ b/internal/controller/artifactgenerator_controller.go @@ -198,7 +198,7 @@ func (r *ArtifactGeneratorReconciler) reconcile(ctx context.Context, // ////.tar.gz artifactBuilder := builder.New(r.Storage) - reqs, err := buildArtifactRequests(obj, localSources) + reqs, err := buildArtifactRequests(ctx, obj, localSources) if err != nil { msg := fmt.Sprintf("failed to expand path pattern: %s", err.Error()) gotkconditions.MarkFalse(obj, diff --git a/internal/controller/artifactgenerator_controller_test.go b/internal/controller/artifactgenerator_controller_test.go index ea5566bb..59631e16 100644 --- a/internal/controller/artifactgenerator_controller_test.go +++ b/internal/controller/artifactgenerator_controller_test.go @@ -877,11 +877,11 @@ func TestArtifactGeneratorReconciler_PathPattern(t *testing.T) { obj.Spec.PathPattern = fmt.Sprintf("@%s-git/apps/{app}/envs/{env}", objKey.Name) obj.Spec.OutputArtifacts = []swapi.OutputArtifact{ { - Name: fmt.Sprintf("%s-{{ .app }}-{{ .env }}", objKey.Name), + Name: fmt.Sprintf("%q + '-' + app + '-' + env", objKey.Name), Copy: []swapi.CopyOperation{ { - From: fmt.Sprintf("@%s-git/apps/{{ .app }}/envs/{{ .env }}/**", objKey.Name), - To: "@artifact/", + From: fmt.Sprintf("'@%s-git/apps/' + app + '/envs/' + env + '/**'", objKey.Name), + To: "'@artifact/'", }, }, }, diff --git a/internal/controller/artifactgenerator_drift_test.go b/internal/controller/artifactgenerator_drift_test.go index 5f7749a4..282787e3 100644 --- a/internal/controller/artifactgenerator_drift_test.go +++ b/internal/controller/artifactgenerator_drift_test.go @@ -334,9 +334,9 @@ func TestArtifactGeneratorReconciler_DetectDrift(t *testing.T) { PathPattern: "@test/apps/{app}/envs/{env}", OutputArtifacts: []swapi.OutputArtifact{ { - Name: "{{ .app }}-{{ .env }}", + Name: "app + '-' + env", Copy: []swapi.CopyOperation{ - {From: "@test/apps/{{ .app }}/envs/{{ .env }}/**", To: "@artifact/"}, + {From: "'@test/apps/' + app + '/envs/' + env + '/**'", To: "'@artifact/'"}, }, }, }, diff --git a/internal/controller/artifactgenerator_pathpattern.go b/internal/controller/artifactgenerator_pathpattern.go index 336ff7fe..5a2d4908 100644 --- a/internal/controller/artifactgenerator_pathpattern.go +++ b/internal/controller/artifactgenerator_pathpattern.go @@ -17,14 +17,14 @@ limitations under the License. package controller import ( - "bytes" + "context" "fmt" "io/fs" "path/filepath" "regexp" "strings" - "text/template" + gotkcel "github.com/fluxcd/pkg/runtime/cel" swapi "github.com/fluxcd/source-watcher/api/v2/v1beta1" "k8s.io/apimachinery/pkg/api/validate/content" apivalidation "k8s.io/apimachinery/pkg/api/validation" @@ -35,11 +35,11 @@ type artifactRequest struct { Labels map[string]string } -var pathPatternCaptureNameRe = regexp.MustCompile("^[A-Za-z0-9_]+$") +var pathPatternCaptureNameRe = regexp.MustCompile("^[A-Za-z_][A-Za-z0-9_]*$") // buildArtifactRequests expands the PathPattern into multiple artifact requests if specified. // Otherwise, it returns the OutputArtifacts exactly as defined in the spec. -func buildArtifactRequests(obj *swapi.ArtifactGenerator, localSources map[string]string) ([]artifactRequest, error) { +func buildArtifactRequests(ctx context.Context, obj *swapi.ArtifactGenerator, localSources map[string]string) ([]artifactRequest, error) { if obj.Spec.PathPattern == "" { reqs := make([]artifactRequest, 0, len(obj.Spec.OutputArtifacts)) for _, oa := range obj.Spec.OutputArtifacts { @@ -140,7 +140,7 @@ func buildArtifactRequests(obj *swapi.ArtifactGenerator, localSources map[string // Render the OutputArtifacts using the captured variables. for _, oa := range obj.Spec.OutputArtifacts { - req, err := renderArtifactRequest(oa, normalizedCaptures, rawCaptures) + req, err := renderArtifactRequest(ctx, oa, normalizedCaptures, rawCaptures) if err != nil { return fmt.Errorf("failed to render artifact %s for match %s: %w", oa.Name, rel, err) } @@ -188,7 +188,10 @@ func validatePathPatternCaptures(pathPattern, patternStr string) error { return fmt.Errorf("pathPattern %q: empty capture variable", pathPattern) } if !pathPatternCaptureNameRe.MatchString(name) { - return fmt.Errorf("pathPattern %q: capture variable %q must match [A-Za-z0-9_]+", pathPattern, name) + return fmt.Errorf("pathPattern %q: capture variable %q must be a valid CEL variable name matching [A-Za-z_][A-Za-z0-9_]*", pathPattern, name) + } + if !isCELStringVariableName(name) { + return fmt.Errorf("pathPattern %q: capture variable %q is not a valid CEL string variable name", pathPattern, name) } if _, ok := seen[name]; ok { return fmt.Errorf("pathPattern %q: duplicate capture variable %q", pathPattern, name) @@ -202,10 +205,10 @@ func validatePathPatternCaptures(pathPattern, patternStr string) error { return nil } -// renderArtifactRequest creates an artifactRequest by evaluating Go template expressions. +// renderArtifactRequest creates an artifactRequest by evaluating CEL expressions. // Artifact names and labels use normalized captures, while copy paths use raw captures. -func renderArtifactRequest(oa swapi.OutputArtifact, normalizedCaptures, rawCaptures map[string]string) (artifactRequest, error) { - name, err := renderTemplate(oa.Name, normalizedCaptures) +func renderArtifactRequest(ctx context.Context, oa swapi.OutputArtifact, normalizedCaptures, rawCaptures map[string]string) (artifactRequest, error) { + name, err := renderCELString(ctx, oa.Name, normalizedCaptures) if err != nil { return artifactRequest{}, err } @@ -217,15 +220,15 @@ func renderArtifactRequest(oa swapi.OutputArtifact, normalizedCaptures, rawCaptu req.Name = name // Deep copy the Copy slice to avoid mutating the original OutputArtifact spec, - // and render template expressions in each copy operation. + // and evaluate CEL expressions in each copy operation. req.Copy = make([]swapi.CopyOperation, len(oa.Copy)) for i, copyOp := range oa.Copy { - from, err := renderTemplate(copyOp.From, rawCaptures) + from, err := renderCELString(ctx, copyOp.From, rawCaptures) if err != nil { return artifactRequest{}, err } - to, err := renderTemplate(copyOp.To, rawCaptures) + to, err := renderCELString(ctx, copyOp.To, rawCaptures) if err != nil { return artifactRequest{}, err } @@ -241,19 +244,27 @@ func renderArtifactRequest(oa swapi.OutputArtifact, normalizedCaptures, rawCaptu return req, nil } -// renderTemplate evaluates a Go template string with the provided data map. -// If the string does not contain any template delimiters, it is returned as-is. -func renderTemplate(tmplStr string, data map[string]string) (string, error) { - if !strings.Contains(tmplStr, "{{") { - return tmplStr, nil - } - tmpl, err := template.New("").Parse(tmplStr) +func renderCELString(ctx context.Context, expr string, data map[string]string) (string, error) { + celExpr, err := gotkcel.NewExpression(expr) if err != nil { return "", err } - var buf bytes.Buffer - if err := tmpl.Execute(&buf, data); err != nil { - return "", err + return celExpr.EvaluateString(ctx, celActivation(data)) +} + +func celActivation(data map[string]string) map[string]any { + activation := make(map[string]any, len(data)) + for k, v := range data { + activation[k] = v + } + return activation +} + +func isCELStringVariableName(name string) bool { + expr, err := gotkcel.NewExpression(name) + if err != nil { + return false } - return buf.String(), nil + got, err := expr.EvaluateString(context.Background(), map[string]any{name: "test"}) + return err == nil && got == "test" } diff --git a/internal/controller/artifactgenerator_pathpattern_test.go b/internal/controller/artifactgenerator_pathpattern_test.go index 4a303c44..c0cfffb7 100644 --- a/internal/controller/artifactgenerator_pathpattern_test.go +++ b/internal/controller/artifactgenerator_pathpattern_test.go @@ -17,6 +17,7 @@ limitations under the License. package controller import ( + "context" "os" "path/filepath" "strings" @@ -41,6 +42,7 @@ func TestBuildArtifactRequests(t *testing.T) { localSources := map[string]string{ "repo": aliasDir, } + ctx := context.Background() createFile := func(path string) { p := filepath.Join(aliasDir, path) @@ -66,7 +68,7 @@ func TestBuildArtifactRequests(t *testing.T) { }, }, } - reqs, err := buildArtifactRequests(obj, localSources) + reqs, err := buildArtifactRequests(ctx, obj, localSources) g.Expect(err).ToNot(gomega.HaveOccurred()) g.Expect(reqs).To(gomega.HaveLen(2)) g.Expect(reqs[0].Name).To(gomega.Equal("static-1")) @@ -80,7 +82,7 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: "invalid-no-at-sign", }, } - _, err := buildArtifactRequests(obj, localSources) + _, err := buildArtifactRequests(ctx, obj, localSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("invalid pathPattern format")) }) @@ -92,7 +94,7 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: "@unknown/apps/{app}", }, } - _, err := buildArtifactRequests(obj, localSources) + _, err := buildArtifactRequests(ctx, obj, localSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("not found in local sources")) }) @@ -104,7 +106,7 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: "@repo/apps/{_app}", }, } - _, err := buildArtifactRequests(obj, localSources) + _, err := buildArtifactRequests(ctx, obj, localSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring("capture variable")) @@ -118,7 +120,8 @@ func TestBuildArtifactRequests(t *testing.T) { wantErrMsg string }{ {name: "empty capture", pattern: "@repo/apps/{}", wantErrMsg: "empty capture variable"}, - {name: "unsupported capture name", pattern: "@repo/apps/{app-name}", wantErrMsg: "must match [A-Za-z0-9_]+"}, + {name: "unsupported capture name", pattern: "@repo/apps/{app-name}", wantErrMsg: "valid CEL variable name"}, + {name: "capture name starts with digit", pattern: "@repo/apps/{1app}", wantErrMsg: "valid CEL variable name"}, {name: "missing closing brace", pattern: "@repo/apps/{app", wantErrMsg: "missing closing brace"}, {name: "missing opening brace", pattern: "@repo/apps/app}", wantErrMsg: "missing opening brace"}, {name: "duplicate capture", pattern: "@repo/apps/{app}/envs/{app}", wantErrMsg: "duplicate capture variable"}, @@ -132,7 +135,7 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: tc.pattern, }, } - _, err := buildArtifactRequests(obj, localSources) + _, err := buildArtifactRequests(ctx, obj, localSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring(tc.pattern)) @@ -148,15 +151,15 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ { - Name: "app-{{ .app }}", + Name: "'app-' + app", Copy: []swapi.CopyOperation{ - {From: "apps/{{ .app }}", To: "."}, + {From: "'apps/' + app", To: "'.'"}, }, }, }, }, } - reqs, err := buildArtifactRequests(obj, localSources) + reqs, err := buildArtifactRequests(ctx, obj, localSources) g.Expect(err).ToNot(gomega.HaveOccurred()) g.Expect(reqs).To(gomega.HaveLen(3)) // auth, payments, and ignore @@ -171,6 +174,22 @@ func TestBuildArtifactRequests(t *testing.T) { } }) + t.Run("missing CEL variable", func(t *testing.T) { + g := gomega.NewWithT(t) + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@repo/apps/{app}", + OutputArtifacts: []swapi.OutputArtifact{ + {Name: "missing"}, + }, + }, + } + _, err := buildArtifactRequests(ctx, obj, localSources) + g.Expect(err).To(gomega.HaveOccurred()) + g.Expect(err.Error()).To(gomega.ContainSubstring("failed to evaluate the CEL expression")) + g.Expect(err.Error()).To(gomega.ContainSubstring("no such attribute")) + }) + t.Run("preserves Exclude and Strategy fields", func(t *testing.T) { g := gomega.NewWithT(t) obj := &swapi.ArtifactGenerator{ @@ -178,11 +197,11 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ { - Name: "app-{{ .app }}", + Name: "'app-' + app", Copy: []swapi.CopyOperation{ { - From: "apps/{{ .app }}", - To: ".", + From: "'apps/' + app", + To: "'.'", Exclude: []string{"*.md", "tests/"}, Strategy: "Merge", }, @@ -191,7 +210,7 @@ func TestBuildArtifactRequests(t *testing.T) { }, }, } - reqs, err := buildArtifactRequests(obj, localSources) + reqs, err := buildArtifactRequests(ctx, obj, localSources) g.Expect(err).ToNot(gomega.HaveOccurred()) g.Expect(reqs).To(gomega.HaveLen(3)) @@ -207,11 +226,11 @@ func TestBuildArtifactRequests(t *testing.T) { Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}/envs/{env}", OutputArtifacts: []swapi.OutputArtifact{ - {Name: "{{ .app }}-{{ .env }}"}, + {Name: "app + '-' + env"}, }, }, } - reqs, err := buildArtifactRequests(obj, localSources) + reqs, err := buildArtifactRequests(ctx, obj, localSources) g.Expect(err).ToNot(gomega.HaveOccurred()) g.Expect(reqs).To(gomega.HaveLen(2)) // only auth/envs/dev and auth/envs/prod, ignore is a file @@ -242,15 +261,15 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: "@repo/apps/{app}/envs/{env}", OutputArtifacts: []swapi.OutputArtifact{ { - Name: "{{ .app }}-{{ .env }}", + Name: "app + '-' + env", Copy: []swapi.CopyOperation{ - {From: "@repo/apps/{{ .app }}/envs/{{ .env }}/**", To: "@artifact/{{ .app }}/{{ .env }}/"}, + {From: "'@repo/apps/' + app + '/envs/' + env + '/**'", To: "'@artifact/' + app + '/' + env + '/'"}, }, }, }, }, } - reqs, err := buildArtifactRequests(obj, upperSources) + reqs, err := buildArtifactRequests(ctx, obj, upperSources) g.Expect(err).ToNot(gomega.HaveOccurred()) g.Expect(reqs).To(gomega.HaveLen(2)) @@ -291,11 +310,11 @@ func TestBuildArtifactRequests(t *testing.T) { Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ - {Name: "app-{{ .app }}"}, + {Name: "'app-' + app"}, }, }, } - _, err = buildArtifactRequests(obj, dotSources) + _, err = buildArtifactRequests(ctx, obj, dotSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes label value")) @@ -317,11 +336,11 @@ func TestBuildArtifactRequests(t *testing.T) { Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ - {Name: "app-{{ .app }}"}, + {Name: "'app-' + app"}, }, }, } - _, err = buildArtifactRequests(obj, spaceSources) + _, err = buildArtifactRequests(ctx, obj, spaceSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes label value")) @@ -344,11 +363,11 @@ func TestBuildArtifactRequests(t *testing.T) { Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ - {Name: "app-{{ .app }}"}, + {Name: "'app-' + app"}, }, }, } - _, err = buildArtifactRequests(obj, longSources) + _, err = buildArtifactRequests(ctx, obj, longSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes label value")) @@ -370,12 +389,12 @@ func TestBuildArtifactRequests(t *testing.T) { Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ - // Template produces "auth..invalid" which is not a valid DNS subdomain. - {Name: "{{ .app }}..invalid"}, + // CEL expression produces "auth..invalid" which is not a valid DNS subdomain. + {Name: "app + '..invalid'"}, }, }, } - _, err = buildArtifactRequests(obj, dnsSources) + _, err = buildArtifactRequests(ctx, obj, dnsSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes object name")) @@ -399,11 +418,11 @@ func TestBuildArtifactRequests(t *testing.T) { Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ - {Name: "app-{{ .app }}"}, + {Name: "'app-' + app"}, }, }, } - _, err = buildArtifactRequests(obj, dupSources) + _, err = buildArtifactRequests(ctx, obj, dupSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring("both resolve to artifact name")) diff --git a/internal/controller/artifactgenerator_validation_test.go b/internal/controller/artifactgenerator_validation_test.go index 751e4ad7..b297bc58 100644 --- a/internal/controller/artifactgenerator_validation_test.go +++ b/internal/controller/artifactgenerator_validation_test.go @@ -248,7 +248,7 @@ func TestArtifactGenerator_crdValidation(t *testing.T) { expectError: true, }, { - name: "templated artifact name with pathPattern", + name: "CEL artifact name with pathPattern", setupObj: func() *swapi.ArtifactGenerator { objKey := client.ObjectKey{ Name: "test-path-pattern-template-name", @@ -257,8 +257,9 @@ func TestArtifactGenerator_crdValidation(t *testing.T) { obj := getArtifactGenerator(objKey) alias := obj.Spec.Sources[0].Alias obj.Spec.PathPattern = "@" + alias + "/apps/{app}" - obj.Spec.OutputArtifacts[0].Name = "{{ .app }}" - obj.Spec.OutputArtifacts[0].Copy[0].From = "@" + alias + "/apps/{{ .app }}/**" + obj.Spec.OutputArtifacts[0].Name = "app" + obj.Spec.OutputArtifacts[0].Copy[0].From = "'@" + alias + "/apps/' + app + '/**'" + obj.Spec.OutputArtifacts[0].Copy[0].To = "'@artifact/file.yaml'" return obj }, expectError: false, From 7b6782d1519f535dfeb54f2a9a23a0c1a8382733 Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Wed, 24 Jun 2026 12:24:26 +0100 Subject: [PATCH 08/10] Fix copy-paste mistake ResourceSet -> ArtifactGenerator Signed-off-by: Matheus Pimenta --- internal/controller/artifactgenerator_finalize_test.go | 4 ++-- internal/controller/artifactgenerator_validation_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/controller/artifactgenerator_finalize_test.go b/internal/controller/artifactgenerator_finalize_test.go index a58f3d05..9dc577c6 100644 --- a/internal/controller/artifactgenerator_finalize_test.go +++ b/internal/controller/artifactgenerator_finalize_test.go @@ -32,7 +32,7 @@ import ( swapi "github.com/fluxcd/source-watcher/api/v2/v1beta1" ) -func TestResourceSetReconciler_Finalize(t *testing.T) { +func TestArtifactGeneratorReconciler_Finalize(t *testing.T) { g := NewWithT(t) reconciler := getArtifactGeneratorReconciler() ctx, cancel := context.WithTimeout(context.Background(), timeout) @@ -85,7 +85,7 @@ func TestResourceSetReconciler_Finalize(t *testing.T) { g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) } -func TestResourceSetReconciler_Finalize_Disabled(t *testing.T) { +func TestArtifactGeneratorReconciler_Finalize_Disabled(t *testing.T) { g := NewWithT(t) reconciler := getArtifactGeneratorReconciler() ctx, cancel := context.WithTimeout(context.Background(), timeout) diff --git a/internal/controller/artifactgenerator_validation_test.go b/internal/controller/artifactgenerator_validation_test.go index b297bc58..157b907d 100644 --- a/internal/controller/artifactgenerator_validation_test.go +++ b/internal/controller/artifactgenerator_validation_test.go @@ -31,7 +31,7 @@ import ( swapi "github.com/fluxcd/source-watcher/api/v2/v1beta1" ) -func TestResourceSetReconciler_specValidation(t *testing.T) { +func TestArtifactGeneratorReconciler_specValidation(t *testing.T) { gt := NewWithT(t) reconciler := getArtifactGeneratorReconciler() ctx, cancel := context.WithTimeout(context.Background(), timeout) From ce55985dcb9de5e923e9d009094fc8f80b71deb6 Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Wed, 24 Jun 2026 13:02:31 +0100 Subject: [PATCH 09/10] Fail terminally on errors caused by building artifact requests via pathPattern Signed-off-by: Matheus Pimenta --- .../artifactgenerator_controller.go | 6 ++ .../artifactgenerator_controller_test.go | 57 +++++++++++++++++++ .../artifactgenerator_pathpattern.go | 43 +++++++++++--- .../artifactgenerator_pathpattern_test.go | 19 +++++++ 4 files changed, 116 insertions(+), 9 deletions(-) diff --git a/internal/controller/artifactgenerator_controller.go b/internal/controller/artifactgenerator_controller.go index 61ea0989..01bbdab9 100644 --- a/internal/controller/artifactgenerator_controller.go +++ b/internal/controller/artifactgenerator_controller.go @@ -201,6 +201,12 @@ func (r *ArtifactGeneratorReconciler) reconcile(ctx context.Context, reqs, err := buildArtifactRequests(ctx, obj, localSources) if err != nil { msg := fmt.Sprintf("failed to expand path pattern: %s", err.Error()) + if isTerminalPathPatternError(err) { + return ctrl.Result{}, r.newTerminalErrorFor(obj, + gotkmeta.BuildFailedReason, + "%s", msg) + } + gotkconditions.Delete(obj, gotkmeta.StalledCondition) gotkconditions.MarkFalse(obj, gotkmeta.ReadyCondition, gotkmeta.BuildFailedReason, diff --git a/internal/controller/artifactgenerator_controller_test.go b/internal/controller/artifactgenerator_controller_test.go index 59631e16..a9b2dd2a 100644 --- a/internal/controller/artifactgenerator_controller_test.go +++ b/internal/controller/artifactgenerator_controller_test.go @@ -531,6 +531,63 @@ func TestArtifactGeneratorReconciler_fetchSources(t *testing.T) { } } +func TestArtifactGeneratorReconciler_PathPatternBuildFailedTerminal(t *testing.T) { + g := NewWithT(t) + reconciler := getArtifactGeneratorReconciler() + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + ns, err := testEnv.CreateNamespace(ctx, "test-pattern-build-failed") + g.Expect(err).ToNot(HaveOccurred()) + + objKey := client.ObjectKey{ + Name: "test-pattern-build-failed", + Namespace: ns.Name, + } + obj := getArtifactGenerator(objKey) + obj.Spec.Sources = []swapi.SourceReference{ + { + Alias: fmt.Sprintf("%s-git", objKey.Name), + Kind: sourcev1.GitRepositoryKind, + Name: objKey.Name, + }, + } + obj.Spec.PathPattern = fmt.Sprintf("@%s-git/apps/{app}", objKey.Name) + obj.Spec.OutputArtifacts = []swapi.OutputArtifact{ + { + Name: fmt.Sprintf("%q + '-' + app", objKey.Name), + Copy: []swapi.CopyOperation{ + {From: fmt.Sprintf("'@%s-git/apps/' + app + '/**'", objKey.Name), To: "'@artifact/'"}, + }, + }, + } + + err = testClient.Create(ctx, obj) + g.Expect(err).ToNot(HaveOccurred()) + + err = applyGitRepository(objKey, "main@sha256:buildfailed", []gotktestsrv.File{ + {Name: "apps/.hidden/app.yaml", Body: "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test"}, + }) + g.Expect(err).ToNot(HaveOccurred()) + + r, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: objKey}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(r.RequeueAfter).To(BeEquivalentTo(time.Millisecond)) + + r, err = reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: objKey}) + g.Expect(err).To(HaveOccurred()) + g.Expect(errors.Is(err, reconcile.TerminalError(nil))).To(BeTrue()) + g.Expect(r.RequeueAfter).To(BeZero()) + + err = testClient.Get(ctx, objKey, obj) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(gotkconditions.IsStalled(obj)).To(BeTrue()) + g.Expect(gotkconditions.GetReason(obj, gotkmeta.ReadyCondition)).To(Equal(gotkmeta.BuildFailedReason)) + g.Expect(gotkconditions.GetMessage(obj, gotkmeta.ReadyCondition)).To(ContainSubstring("failed to expand path pattern")) + g.Expect(gotkconditions.GetMessage(obj, gotkmeta.ReadyCondition)).To(ContainSubstring("pathPattern")) + g.Expect(gotkconditions.GetMessage(obj, gotkmeta.ReadyCondition)).To(ContainSubstring("not a valid Kubernetes label value")) +} + func TestArtifactGeneratorReconciler_CommonMetadata(t *testing.T) { g := NewWithT(t) reconciler := getArtifactGeneratorReconciler() diff --git a/internal/controller/artifactgenerator_pathpattern.go b/internal/controller/artifactgenerator_pathpattern.go index 5a2d4908..6b0e3159 100644 --- a/internal/controller/artifactgenerator_pathpattern.go +++ b/internal/controller/artifactgenerator_pathpattern.go @@ -18,6 +18,7 @@ package controller import ( "context" + "errors" "fmt" "io/fs" "path/filepath" @@ -37,6 +38,27 @@ type artifactRequest struct { var pathPatternCaptureNameRe = regexp.MustCompile("^[A-Za-z_][A-Za-z0-9_]*$") +type terminalPathPatternError struct { + err error +} + +func (e *terminalPathPatternError) Error() string { + return e.err.Error() +} + +func (e *terminalPathPatternError) Unwrap() error { + return e.err +} + +func newTerminalPathPatternError(format string, args ...any) error { + return &terminalPathPatternError{err: fmt.Errorf(format, args...)} +} + +func isTerminalPathPatternError(err error) bool { + var terminalErr *terminalPathPatternError + return errors.As(err, &terminalErr) +} + // buildArtifactRequests expands the PathPattern into multiple artifact requests if specified. // Otherwise, it returns the OutputArtifacts exactly as defined in the spec. func buildArtifactRequests(ctx context.Context, obj *swapi.ArtifactGenerator, localSources map[string]string) ([]artifactRequest, error) { @@ -54,7 +76,7 @@ func buildArtifactRequests(ctx context.Context, obj *swapi.ArtifactGenerator, lo // Parse @/ parts := strings.SplitN(obj.Spec.PathPattern, "/", 2) if len(parts) != 2 || !strings.HasPrefix(parts[0], "@") { - return nil, fmt.Errorf("invalid pathPattern format: %s", obj.Spec.PathPattern) + return nil, newTerminalPathPatternError("invalid pathPattern format: %s", obj.Spec.PathPattern) } alias := strings.TrimPrefix(parts[0], "@") @@ -62,27 +84,27 @@ func buildArtifactRequests(ctx context.Context, obj *swapi.ArtifactGenerator, lo srcDir, ok := localSources[alias] if !ok { - return nil, fmt.Errorf("source alias %s not found in local sources", alias) + return nil, newTerminalPathPatternError("pathPattern %q: source alias %s not found in local sources", obj.Spec.PathPattern, alias) } // Convert the path pattern (e.g. "apps/{app}/envs/{env}") to a regex // with named capture groups (e.g. "^apps/(?P[^/]+)/envs/(?P[^/]+)$"). escaped := regexp.QuoteMeta(patternStr) if err := validatePathPatternCaptures(obj.Spec.PathPattern, patternStr); err != nil { - return nil, err + return nil, newTerminalPathPatternError("%w", err) } captureRe := regexp.MustCompile(`\\\{([a-zA-Z0-9_]+)\\\}`) regexStr := "^" + captureRe.ReplaceAllString(escaped, `(?P<$1>[^/]+)`) + "$" matcher, err := regexp.Compile(regexStr) if err != nil { - return nil, fmt.Errorf("failed to compile path pattern: %w", err) + return nil, newTerminalPathPatternError("pathPattern %q: failed to compile path pattern: %w", obj.Spec.PathPattern, err) } subexpNames := matcher.SubexpNames() for _, name := range subexpNames[1:] { if errs := content.IsLabelKey(name); len(errs) > 0 { - return nil, fmt.Errorf( + return nil, newTerminalPathPatternError( "pathPattern %q: capture variable %q is not a valid Kubernetes label key: %s", obj.Spec.PathPattern, name, strings.Join(errs, "; ")) } @@ -132,7 +154,7 @@ func buildArtifactRequests(ctx context.Context, obj *swapi.ArtifactGenerator, lo // Validate values with content.IsLabelValue. for k, v := range normalizedCaptures { if errs := content.IsLabelValue(v); len(errs) > 0 { - return fmt.Errorf( + return newTerminalPathPatternError( "pathPattern %q: captured value %q for variable %q is not a valid Kubernetes label value: %s", obj.Spec.PathPattern, v, k, strings.Join(errs, "; ")) } @@ -142,19 +164,19 @@ func buildArtifactRequests(ctx context.Context, obj *swapi.ArtifactGenerator, lo for _, oa := range obj.Spec.OutputArtifacts { req, err := renderArtifactRequest(ctx, oa, normalizedCaptures, rawCaptures) if err != nil { - return fmt.Errorf("failed to render artifact %s for match %s: %w", oa.Name, rel, err) + return newTerminalPathPatternError("pathPattern %q: failed to render artifact %s for match %s: %w", obj.Spec.PathPattern, oa.Name, rel, err) } // Validate rendered EA name with NameIsDNSSubdomain. if errs := apivalidation.NameIsDNSSubdomain(req.Name, false); len(errs) > 0 { - return fmt.Errorf( + return newTerminalPathPatternError( "pathPattern %q: rendered artifact name %q is not a valid Kubernetes object name: %s", obj.Spec.PathPattern, req.Name, strings.Join(errs, "; ")) } // Validate for duplicate names. if prevDir, exists := seenNames[req.Name]; exists { - return fmt.Errorf( + return newTerminalPathPatternError( "pathPattern %q: directories %q and %q both resolve to artifact name %q", obj.Spec.PathPattern, prevDir, rel, req.Name) } @@ -167,6 +189,9 @@ func buildArtifactRequests(ctx context.Context, obj *swapi.ArtifactGenerator, lo }) if err != nil { + if isTerminalPathPatternError(err) { + return nil, err + } return nil, fmt.Errorf("failed to walk source directory: %w", err) } diff --git a/internal/controller/artifactgenerator_pathpattern_test.go b/internal/controller/artifactgenerator_pathpattern_test.go index c0cfffb7..b1c5e090 100644 --- a/internal/controller/artifactgenerator_pathpattern_test.go +++ b/internal/controller/artifactgenerator_pathpattern_test.go @@ -108,11 +108,30 @@ func TestBuildArtifactRequests(t *testing.T) { } _, err := buildArtifactRequests(ctx, obj, localSources) g.Expect(err).To(gomega.HaveOccurred()) + g.Expect(isTerminalPathPatternError(err)).To(gomega.BeTrue()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring("capture variable")) g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes label key")) }) + t.Run("source walk error is not terminal", func(t *testing.T) { + g := gomega.NewWithT(t) + obj := &swapi.ArtifactGenerator{ + Spec: swapi.ArtifactGeneratorSpec{ + PathPattern: "@repo/apps/{app}", + OutputArtifacts: []swapi.OutputArtifact{ + {Name: "'app-' + app"}, + }, + }, + } + _, err := buildArtifactRequests(ctx, obj, map[string]string{ + "repo": filepath.Join(tmpDir, "does-not-exist"), + }) + g.Expect(err).To(gomega.HaveOccurred()) + g.Expect(isTerminalPathPatternError(err)).To(gomega.BeFalse()) + g.Expect(err.Error()).To(gomega.ContainSubstring("failed to walk source directory")) + }) + t.Run("invalid capture syntax", func(t *testing.T) { cases := []struct { name string From bcba2c6fcbbbe1c37db4a5e3a2b53037a5d31061 Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Wed, 24 Jun 2026 14:46:31 +0100 Subject: [PATCH 10/10] Change pathPattern CEL expressions to the same template syntax as pathPattern Signed-off-by: Matheus Pimenta --- api/v1beta1/artifactgenerator_types.go | 12 +-- ...tensions.fluxcd.io_artifactgenerators.yaml | 12 +-- docs/spec/v1beta1/artifactgenerators.md | 17 ++--- go.mod | 8 -- go.sum | 17 ----- .../artifactgenerator_controller.go | 2 +- .../artifactgenerator_controller_test.go | 10 +-- .../artifactgenerator_drift_test.go | 4 +- .../artifactgenerator_pathpattern.go | 73 +++++++++--------- .../artifactgenerator_pathpattern_test.go | 75 +++++++++---------- .../artifactgenerator_validation_test.go | 8 +- 11 files changed, 105 insertions(+), 133 deletions(-) diff --git a/api/v1beta1/artifactgenerator_types.go b/api/v1beta1/artifactgenerator_types.go index 5554a0c4..ea284322 100644 --- a/api/v1beta1/artifactgenerator_types.go +++ b/api/v1beta1/artifactgenerator_types.go @@ -71,7 +71,7 @@ type ArtifactGeneratorSpec struct { // PathPattern specifies a directory traversal pattern to match within the sources. // The format is "@/". Named captures in the pattern (e.g. "{app}") - // are injected as string variables into OutputArtifacts CEL expressions. + // can be used as placeholders in OutputArtifacts fields. // +kubebuilder:validation:Pattern="^@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)$" // +kubebuilder:validation:MaxLength=1024 // +optional @@ -119,7 +119,7 @@ type SourceReference struct { // generated by the ArtifactGenerator. type OutputArtifact struct { // Name is the name of the generated artifact. - // When pathPattern is set, this field is evaluated as a CEL string expression. + // When pathPattern is set, this field may use capture placeholders such as "{app}". // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=253 // +required @@ -155,8 +155,8 @@ type OutputArtifact struct { type CopyOperation struct { // From specifies the source (by alias) and the glob pattern to match files. // The format is "@/". When pathPattern is set, - // this field is evaluated as a CEL string expression. - // +kubebuilder:validation:Pattern="^(@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)|.*['\"+].*)$" + // the path may use capture placeholders such as "{app}". + // +kubebuilder:validation:Pattern="^@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)$" // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=1024 // +required @@ -165,8 +165,8 @@ type CopyOperation struct { // To specifies the destination path within the artifact. // The format is "@artifact/path", the alias "artifact" // refers to the root path of the generated artifact. When pathPattern - // is set, this field is evaluated as a CEL string expression. - // +kubebuilder:validation:Pattern="^(@(artifact)/(.*)|.*['\"+].*)$" + // is set, the path may use capture placeholders such as "{app}". + // +kubebuilder:validation:Pattern="^@(artifact)/(.*)$" // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=1024 // +required diff --git a/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml b/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml index e231f830..a131dbe7 100644 --- a/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml +++ b/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml @@ -80,10 +80,10 @@ spec: description: |- From specifies the source (by alias) and the glob pattern to match files. The format is "@/". When pathPattern is set, - this field is evaluated as a CEL string expression. + the path may use capture placeholders such as "{app}". maxLength: 1024 minLength: 1 - pattern: ^(@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)|.*['"+].*)$ + pattern: ^@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)$ type: string strategy: description: |- @@ -104,10 +104,10 @@ spec: To specifies the destination path within the artifact. The format is "@artifact/path", the alias "artifact" refers to the root path of the generated artifact. When pathPattern - is set, this field is evaluated as a CEL string expression. + is set, the path may use capture placeholders such as "{app}". maxLength: 1024 minLength: 1 - pattern: ^(@(artifact)/(.*)|.*['"+].*)$ + pattern: ^@(artifact)/(.*)$ type: string required: - from @@ -118,7 +118,7 @@ spec: name: description: |- Name is the name of the generated artifact. - When pathPattern is set, this field is evaluated as a CEL string expression. + When pathPattern is set, this field may use capture placeholders such as "{app}". maxLength: 253 minLength: 1 type: string @@ -169,7 +169,7 @@ spec: description: |- PathPattern specifies a directory traversal pattern to match within the sources. The format is "@/". Named captures in the pattern (e.g. "{app}") - are injected as string variables into OutputArtifacts CEL expressions. + can be used as placeholders in OutputArtifacts fields. maxLength: 1024 pattern: ^@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)$ type: string diff --git a/docs/spec/v1beta1/artifactgenerators.md b/docs/spec/v1beta1/artifactgenerators.md index 90cf033a..dc5992e5 100644 --- a/docs/spec/v1beta1/artifactgenerators.md +++ b/docs/spec/v1beta1/artifactgenerators.md @@ -232,8 +232,7 @@ regenerate the affected artifacts automatically. The `.spec.pathPattern` field allows for dynamic, path-based discovery of artifacts. When specified, the controller will scan the referenced source for directories matching the pattern and dynamically generate one ExternalArtifact for each matched directory. - `pathPattern` (optional): Specifies a directory traversal pattern within a source in the format `@/`. - Named captures in the pattern (e.g., `{app}`, `{env}`) are injected as string variables into CEL expressions in the `artifacts` fields. - The `pathPattern` itself is not a CEL expression. + Named captures in the pattern (e.g., `{app}`, `{env}`) can be used as placeholders in the `artifacts` fields. When `pathPattern` is used, the generated ExternalArtifacts will automatically have their labels populated with the extracted capture variables. @@ -245,10 +244,10 @@ spec: name: my-monorepo pathPattern: "@monorepo/apps/{app}/envs/{env}" artifacts: - - name: "app + '-' + env" + - name: "{app}-{env}" copy: - - from: "'@monorepo/apps/' + app + '/envs/' + env + '/**'" - to: "'@artifact/'" + - from: "@monorepo/apps/{app}/envs/{env}/**" + to: "@artifact/" ``` #### Directory Name Constraints @@ -277,11 +276,11 @@ the `pathPattern` and the invalid value. ### Artifacts The `.spec.artifacts` field defines the list of ExternalArtifacts to be generated from the sources. -When `pathPattern` is defined, the artifacts act as expression templates for each matched directory. +When `pathPattern` is defined, the artifacts act as templates for each matched directory. Each artifact must specify: - `name` (required): The name of the generated ExternalArtifact resource. It must be unique in the context - of the ArtifactGenerator and must conform to Kubernetes resource naming conventions. Supports CEL expressions if `pathPattern` is used. + of the ArtifactGenerator and must conform to Kubernetes resource naming conventions. Supports capture placeholders if `pathPattern` is used. - `copy` (required): A list of copy operations to perform from sources to the artifact. - `revision` (optional): A specific source revision to use in the format `@alias`. If not specified, the revision is automatically computed as `latest@` based on the artifact content. @@ -312,10 +311,10 @@ Each copy operation specifies how to copy files from sources into the generated - `from`: Source path in the format `@alias/pattern` where `alias` references a source and `pattern` is a glob pattern or a specific file/directory path within that source. - When `pathPattern` is set, this field is a CEL string expression that must evaluate to this format. + When `pathPattern` is set, this field may use capture placeholders and must render to this format. - `to`: Destination path in the format `@artifact/path` where `artifact` is the root of the generated artifact and `path` is the relative path to a file or directory. - When `pathPattern` is set, this field is a CEL string expression that must evaluate to this format. + When `pathPattern` is set, this field may use capture placeholders and must render to this format. - `exclude` (optional): A list of glob patterns to filter out from the source selection. Any file matched by `from` that also matches an exclude pattern will be ignored. Patterns are matched against paths relative to the source alias root or to the diff --git a/go.mod b/go.mod index 1f8d65a7..8f5365c1 100644 --- a/go.mod +++ b/go.mod @@ -32,11 +32,9 @@ require ( ) require ( - cel.dev/expr v0.25.1 // indirect github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect github.com/MakeNowJust/heredoc v1.0.0 // indirect github.com/Masterminds/semver/v3 v3.5.0 // indirect - github.com/antlr4-go/antlr/v4 v4.13.0 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect @@ -50,7 +48,6 @@ require ( github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f // indirect github.com/fluxcd/cli-utils v1.2.1 // indirect github.com/fluxcd/pkg/apis/acl v0.10.0 // indirect - github.com/fluxcd/pkg/apis/kustomize v1.19.0 // indirect github.com/fluxcd/pkg/lockedfile v0.8.0 // indirect github.com/fluxcd/pkg/oci v0.68.0 // indirect github.com/fluxcd/pkg/sourceignore v0.18.0 // indirect @@ -75,7 +72,6 @@ require ( github.com/go-openapi/swag/typeutils v0.25.4 // indirect github.com/go-openapi/swag/yamlutils v0.25.4 // indirect github.com/google/btree v1.1.3 // indirect - github.com/google/cel-go v0.26.1 // indirect github.com/google/gnostic-models v0.7.0 // indirect github.com/google/go-cmp v0.7.0 // indirect github.com/google/go-containerregistry v0.21.6 // indirect @@ -105,7 +101,6 @@ require ( github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/sirupsen/logrus v1.9.4 // indirect github.com/spf13/cobra v1.10.2 // indirect - github.com/stoewer/go-strcase v1.3.0 // indirect github.com/x448/float16 v0.8.4 // indirect github.com/xlab/treeprint v1.2.0 // indirect github.com/zeebo/blake3 v0.2.4 // indirect @@ -113,7 +108,6 @@ require ( go.uber.org/zap v1.27.1 // indirect go.yaml.in/yaml/v2 v2.4.4 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93 // indirect golang.org/x/net v0.55.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sync v0.20.0 // indirect @@ -122,8 +116,6 @@ require ( golang.org/x/text v0.37.0 // indirect golang.org/x/time v0.15.0 // indirect gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect - google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9 // indirect google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af // indirect gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect diff --git a/go.sum b/go.sum index 7dc6fcb9..5e13839c 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,3 @@ -cel.dev/expr v0.25.1 h1:1KrZg61W6TWSxuNZ37Xy49ps13NUovb66QLprthtwi4= -cel.dev/expr v0.25.1/go.mod h1:hrXvqGP6G6gyx8UAHSHJ5RGk//1Oj5nXQ2NI02Nrsg4= github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6 h1:He8afgbRMd7mFxO99hRNu+6tazq8nFF9lIwo9JFroBk= github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6/go.mod h1:8o94RPi1/7XTJvwPpRSzSUedZrtlirdB3r9Z20bi2f8= github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 h1:L/gRVlceqvL25UVaW/CKtUDjefjrs0SPonmDGUVOYP0= @@ -8,8 +6,6 @@ github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE= github.com/Masterminds/semver/v3 v3.5.0 h1:kQceYJfbupGfZOKZQg0kou0DgAKhzDg2NZPAwZ/2OOE= github.com/Masterminds/semver/v3 v3.5.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= -github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI= -github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= @@ -67,8 +63,6 @@ github.com/fluxcd/pkg/apis/acl v0.10.0 h1:KPfAmELNvtvaz8wixnm/MYXqa+MJf7ntVVMUU9 github.com/fluxcd/pkg/apis/acl v0.10.0/go.mod h1:a87i2A7AlFO5N2J8CxtzaUCCDmuLLWOHwkKu3eJF5fY= github.com/fluxcd/pkg/apis/event v0.27.0 h1:fUJRyWU3sEKjV6SzBnoJT6aDP5cJdMAbspZyRfhde6I= github.com/fluxcd/pkg/apis/event v0.27.0/go.mod h1:0zua8dB0E9SXryScEf7LDMuUYVyVKgA/Xeh2h2gtSRU= -github.com/fluxcd/pkg/apis/kustomize v1.19.0 h1:DKiRUwS/8HaePXUF+JUShbBe/HAClnJwm2tdiUnPMiY= -github.com/fluxcd/pkg/apis/kustomize v1.19.0/go.mod h1:BYPKRhf3MQKPtSsaNcOs44BigF0q8Fzza5iuOHTd26U= github.com/fluxcd/pkg/apis/meta v1.30.0 h1:26TOd1hbamH3c5KOb/CIMGpUDB4G4JV+WCcPyUhmuaM= github.com/fluxcd/pkg/apis/meta v1.30.0/go.mod h1:q1YjUeCmf0syhkZoMcRmP3pkBaLKFLl7g0mUvVGG4CM= github.com/fluxcd/pkg/artifact v0.18.0 h1:TrIcQ6TaQ+787IA3YESFnihPqbfptqHHyqHZ2y0Cnsk= @@ -141,8 +135,6 @@ github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1v github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= github.com/google/btree v1.1.3 h1:CVpQJjYgC4VbzxeGVHfvZrv1ctoYCAI8vbl07Fcxlyg= github.com/google/btree v1.1.3/go.mod h1:qOPhT0dTNdNzV6Z/lhRX0YXUafgPLFUh+gZMl761Gm4= -github.com/google/cel-go v0.26.1 h1:iPbVVEdkhTX++hpe3lzSk7D3G3QSYqLGoHOcEio+UXQ= -github.com/google/cel-go v0.26.1/go.mod h1:A9O8OU9rdvrK5MQyrqfIxo1a0u4g3sF8KB6PUIaryMM= github.com/google/gnostic-models v0.7.0 h1:qwTtogB15McXDaNqTZdzPJRHvaVJlAl+HVQnLmJEJxo= github.com/google/gnostic-models v0.7.0/go.mod h1:whL5G0m6dmc5cPxKc5bdKdEN3UjI7OUGxBlw57miDrQ= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= @@ -252,19 +244,12 @@ github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiT github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= -github.com/stoewer/go-strcase v1.3.0 h1:g0eASXYtp+yvN9fK8sH94oCIk0fau9uV1/ZdJ0AVEzs= -github.com/stoewer/go-strcase v1.3.0/go.mod h1:fAH5hQ5pehh+j3nZfvwdk2RgEgQjAoM8wodgtPmh1xo= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= -github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= -github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= @@ -335,8 +320,6 @@ go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI= golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8= -golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93 h1:fQsdNF2N+/YewlRZiricy4P1iimyPKZ/xwniHj8Q2a0= -golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93/go.mod h1:EPRbTFwzwjXj9NpYyyrvenVh9Y+GFeEvMNh7Xuz7xgU= golang.org/x/mod v0.36.0 h1:JJjpVx6myfUsUdAzZuOSTTmRE0PfZeNWzzvKrP7amb4= golang.org/x/mod v0.36.0/go.mod h1:moc6ELqsWcOw5Ef3xVprK5ul/MvtVvkIXLziUOICjUQ= golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= diff --git a/internal/controller/artifactgenerator_controller.go b/internal/controller/artifactgenerator_controller.go index 01bbdab9..dc6c1404 100644 --- a/internal/controller/artifactgenerator_controller.go +++ b/internal/controller/artifactgenerator_controller.go @@ -198,7 +198,7 @@ func (r *ArtifactGeneratorReconciler) reconcile(ctx context.Context, // ////.tar.gz artifactBuilder := builder.New(r.Storage) - reqs, err := buildArtifactRequests(ctx, obj, localSources) + reqs, err := buildArtifactRequests(obj, localSources) if err != nil { msg := fmt.Sprintf("failed to expand path pattern: %s", err.Error()) if isTerminalPathPatternError(err) { diff --git a/internal/controller/artifactgenerator_controller_test.go b/internal/controller/artifactgenerator_controller_test.go index a9b2dd2a..3c5fe231 100644 --- a/internal/controller/artifactgenerator_controller_test.go +++ b/internal/controller/artifactgenerator_controller_test.go @@ -555,9 +555,9 @@ func TestArtifactGeneratorReconciler_PathPatternBuildFailedTerminal(t *testing.T obj.Spec.PathPattern = fmt.Sprintf("@%s-git/apps/{app}", objKey.Name) obj.Spec.OutputArtifacts = []swapi.OutputArtifact{ { - Name: fmt.Sprintf("%q + '-' + app", objKey.Name), + Name: fmt.Sprintf("%s-{app}", objKey.Name), Copy: []swapi.CopyOperation{ - {From: fmt.Sprintf("'@%s-git/apps/' + app + '/**'", objKey.Name), To: "'@artifact/'"}, + {From: fmt.Sprintf("@%s-git/apps/{app}/**", objKey.Name), To: "@artifact/"}, }, }, } @@ -934,11 +934,11 @@ func TestArtifactGeneratorReconciler_PathPattern(t *testing.T) { obj.Spec.PathPattern = fmt.Sprintf("@%s-git/apps/{app}/envs/{env}", objKey.Name) obj.Spec.OutputArtifacts = []swapi.OutputArtifact{ { - Name: fmt.Sprintf("%q + '-' + app + '-' + env", objKey.Name), + Name: fmt.Sprintf("%s-{app}-{env}", objKey.Name), Copy: []swapi.CopyOperation{ { - From: fmt.Sprintf("'@%s-git/apps/' + app + '/envs/' + env + '/**'", objKey.Name), - To: "'@artifact/'", + From: fmt.Sprintf("@%s-git/apps/{app}/envs/{env}/**", objKey.Name), + To: "@artifact/", }, }, }, diff --git a/internal/controller/artifactgenerator_drift_test.go b/internal/controller/artifactgenerator_drift_test.go index 282787e3..8f740b18 100644 --- a/internal/controller/artifactgenerator_drift_test.go +++ b/internal/controller/artifactgenerator_drift_test.go @@ -334,9 +334,9 @@ func TestArtifactGeneratorReconciler_DetectDrift(t *testing.T) { PathPattern: "@test/apps/{app}/envs/{env}", OutputArtifacts: []swapi.OutputArtifact{ { - Name: "app + '-' + env", + Name: "{app}-{env}", Copy: []swapi.CopyOperation{ - {From: "'@test/apps/' + app + '/envs/' + env + '/**'", To: "'@artifact/'"}, + {From: "@test/apps/{app}/envs/{env}/**", To: "@artifact/"}, }, }, }, diff --git a/internal/controller/artifactgenerator_pathpattern.go b/internal/controller/artifactgenerator_pathpattern.go index 6b0e3159..fe3f1bfa 100644 --- a/internal/controller/artifactgenerator_pathpattern.go +++ b/internal/controller/artifactgenerator_pathpattern.go @@ -17,7 +17,6 @@ limitations under the License. package controller import ( - "context" "errors" "fmt" "io/fs" @@ -25,7 +24,6 @@ import ( "regexp" "strings" - gotkcel "github.com/fluxcd/pkg/runtime/cel" swapi "github.com/fluxcd/source-watcher/api/v2/v1beta1" "k8s.io/apimachinery/pkg/api/validate/content" apivalidation "k8s.io/apimachinery/pkg/api/validation" @@ -61,7 +59,7 @@ func isTerminalPathPatternError(err error) bool { // buildArtifactRequests expands the PathPattern into multiple artifact requests if specified. // Otherwise, it returns the OutputArtifacts exactly as defined in the spec. -func buildArtifactRequests(ctx context.Context, obj *swapi.ArtifactGenerator, localSources map[string]string) ([]artifactRequest, error) { +func buildArtifactRequests(obj *swapi.ArtifactGenerator, localSources map[string]string) ([]artifactRequest, error) { if obj.Spec.PathPattern == "" { reqs := make([]artifactRequest, 0, len(obj.Spec.OutputArtifacts)) for _, oa := range obj.Spec.OutputArtifacts { @@ -162,7 +160,7 @@ func buildArtifactRequests(ctx context.Context, obj *swapi.ArtifactGenerator, lo // Render the OutputArtifacts using the captured variables. for _, oa := range obj.Spec.OutputArtifacts { - req, err := renderArtifactRequest(ctx, oa, normalizedCaptures, rawCaptures) + req, err := renderArtifactRequest(oa, normalizedCaptures, rawCaptures) if err != nil { return newTerminalPathPatternError("pathPattern %q: failed to render artifact %s for match %s: %w", obj.Spec.PathPattern, oa.Name, rel, err) } @@ -213,10 +211,7 @@ func validatePathPatternCaptures(pathPattern, patternStr string) error { return fmt.Errorf("pathPattern %q: empty capture variable", pathPattern) } if !pathPatternCaptureNameRe.MatchString(name) { - return fmt.Errorf("pathPattern %q: capture variable %q must be a valid CEL variable name matching [A-Za-z_][A-Za-z0-9_]*", pathPattern, name) - } - if !isCELStringVariableName(name) { - return fmt.Errorf("pathPattern %q: capture variable %q is not a valid CEL string variable name", pathPattern, name) + return fmt.Errorf("pathPattern %q: capture variable %q must be a valid capture name matching [A-Za-z_][A-Za-z0-9_]*", pathPattern, name) } if _, ok := seen[name]; ok { return fmt.Errorf("pathPattern %q: duplicate capture variable %q", pathPattern, name) @@ -230,10 +225,10 @@ func validatePathPatternCaptures(pathPattern, patternStr string) error { return nil } -// renderArtifactRequest creates an artifactRequest by evaluating CEL expressions. +// renderArtifactRequest creates an artifactRequest by substituting capture placeholders. // Artifact names and labels use normalized captures, while copy paths use raw captures. -func renderArtifactRequest(ctx context.Context, oa swapi.OutputArtifact, normalizedCaptures, rawCaptures map[string]string) (artifactRequest, error) { - name, err := renderCELString(ctx, oa.Name, normalizedCaptures) +func renderArtifactRequest(oa swapi.OutputArtifact, normalizedCaptures, rawCaptures map[string]string) (artifactRequest, error) { + name, err := renderTemplateString(oa.Name, normalizedCaptures) if err != nil { return artifactRequest{}, err } @@ -245,15 +240,15 @@ func renderArtifactRequest(ctx context.Context, oa swapi.OutputArtifact, normali req.Name = name // Deep copy the Copy slice to avoid mutating the original OutputArtifact spec, - // and evaluate CEL expressions in each copy operation. + // and substitute capture placeholders in each copy operation. req.Copy = make([]swapi.CopyOperation, len(oa.Copy)) for i, copyOp := range oa.Copy { - from, err := renderCELString(ctx, copyOp.From, rawCaptures) + from, err := renderTemplateString(copyOp.From, rawCaptures) if err != nil { return artifactRequest{}, err } - to, err := renderCELString(ctx, copyOp.To, rawCaptures) + to, err := renderTemplateString(copyOp.To, rawCaptures) if err != nil { return artifactRequest{}, err } @@ -269,27 +264,33 @@ func renderArtifactRequest(ctx context.Context, oa swapi.OutputArtifact, normali return req, nil } -func renderCELString(ctx context.Context, expr string, data map[string]string) (string, error) { - celExpr, err := gotkcel.NewExpression(expr) - if err != nil { - return "", err - } - return celExpr.EvaluateString(ctx, celActivation(data)) -} - -func celActivation(data map[string]string) map[string]any { - activation := make(map[string]any, len(data)) - for k, v := range data { - activation[k] = v - } - return activation -} - -func isCELStringVariableName(name string) bool { - expr, err := gotkcel.NewExpression(name) - if err != nil { - return false +func renderTemplateString(tmpl string, data map[string]string) (string, error) { + var b strings.Builder + for i := 0; i < len(tmpl); i++ { + switch tmpl[i] { + case '{': + end := strings.IndexByte(tmpl[i+1:], '}') + if end < 0 { + return "", fmt.Errorf("invalid template %q: capture starting at offset %d is missing closing brace", tmpl, i) + } + name := tmpl[i+1 : i+1+end] + if name == "" { + return "", fmt.Errorf("invalid template %q: empty capture variable", tmpl) + } + if !pathPatternCaptureNameRe.MatchString(name) { + return "", fmt.Errorf("invalid template %q: capture variable %q must be a valid capture name matching [A-Za-z_][A-Za-z0-9_]*", tmpl, name) + } + value, ok := data[name] + if !ok { + return "", fmt.Errorf("invalid template %q: capture variable %q is not defined by pathPattern", tmpl, name) + } + b.WriteString(value) + i += end + 1 + case '}': + return "", fmt.Errorf("invalid template %q: capture ending at offset %d is missing opening brace", tmpl, i) + default: + b.WriteByte(tmpl[i]) + } } - got, err := expr.EvaluateString(context.Background(), map[string]any{name: "test"}) - return err == nil && got == "test" + return b.String(), nil } diff --git a/internal/controller/artifactgenerator_pathpattern_test.go b/internal/controller/artifactgenerator_pathpattern_test.go index b1c5e090..ce1e3b5e 100644 --- a/internal/controller/artifactgenerator_pathpattern_test.go +++ b/internal/controller/artifactgenerator_pathpattern_test.go @@ -17,7 +17,6 @@ limitations under the License. package controller import ( - "context" "os" "path/filepath" "strings" @@ -42,7 +41,6 @@ func TestBuildArtifactRequests(t *testing.T) { localSources := map[string]string{ "repo": aliasDir, } - ctx := context.Background() createFile := func(path string) { p := filepath.Join(aliasDir, path) @@ -68,7 +66,7 @@ func TestBuildArtifactRequests(t *testing.T) { }, }, } - reqs, err := buildArtifactRequests(ctx, obj, localSources) + reqs, err := buildArtifactRequests(obj, localSources) g.Expect(err).ToNot(gomega.HaveOccurred()) g.Expect(reqs).To(gomega.HaveLen(2)) g.Expect(reqs[0].Name).To(gomega.Equal("static-1")) @@ -82,7 +80,7 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: "invalid-no-at-sign", }, } - _, err := buildArtifactRequests(ctx, obj, localSources) + _, err := buildArtifactRequests(obj, localSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("invalid pathPattern format")) }) @@ -94,7 +92,7 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: "@unknown/apps/{app}", }, } - _, err := buildArtifactRequests(ctx, obj, localSources) + _, err := buildArtifactRequests(obj, localSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("not found in local sources")) }) @@ -106,7 +104,7 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: "@repo/apps/{_app}", }, } - _, err := buildArtifactRequests(ctx, obj, localSources) + _, err := buildArtifactRequests(obj, localSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(isTerminalPathPatternError(err)).To(gomega.BeTrue()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) @@ -120,11 +118,11 @@ func TestBuildArtifactRequests(t *testing.T) { Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ - {Name: "'app-' + app"}, + {Name: "app-{app}"}, }, }, } - _, err := buildArtifactRequests(ctx, obj, map[string]string{ + _, err := buildArtifactRequests(obj, map[string]string{ "repo": filepath.Join(tmpDir, "does-not-exist"), }) g.Expect(err).To(gomega.HaveOccurred()) @@ -139,8 +137,8 @@ func TestBuildArtifactRequests(t *testing.T) { wantErrMsg string }{ {name: "empty capture", pattern: "@repo/apps/{}", wantErrMsg: "empty capture variable"}, - {name: "unsupported capture name", pattern: "@repo/apps/{app-name}", wantErrMsg: "valid CEL variable name"}, - {name: "capture name starts with digit", pattern: "@repo/apps/{1app}", wantErrMsg: "valid CEL variable name"}, + {name: "unsupported capture name", pattern: "@repo/apps/{app-name}", wantErrMsg: "valid capture name"}, + {name: "capture name starts with digit", pattern: "@repo/apps/{1app}", wantErrMsg: "valid capture name"}, {name: "missing closing brace", pattern: "@repo/apps/{app", wantErrMsg: "missing closing brace"}, {name: "missing opening brace", pattern: "@repo/apps/app}", wantErrMsg: "missing opening brace"}, {name: "duplicate capture", pattern: "@repo/apps/{app}/envs/{app}", wantErrMsg: "duplicate capture variable"}, @@ -154,7 +152,7 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: tc.pattern, }, } - _, err := buildArtifactRequests(ctx, obj, localSources) + _, err := buildArtifactRequests(obj, localSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring(tc.pattern)) @@ -170,15 +168,15 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ { - Name: "'app-' + app", + Name: "app-{app}", Copy: []swapi.CopyOperation{ - {From: "'apps/' + app", To: "'.'"}, + {From: "apps/{app}", To: "."}, }, }, }, }, } - reqs, err := buildArtifactRequests(ctx, obj, localSources) + reqs, err := buildArtifactRequests(obj, localSources) g.Expect(err).ToNot(gomega.HaveOccurred()) g.Expect(reqs).To(gomega.HaveLen(3)) // auth, payments, and ignore @@ -193,20 +191,19 @@ func TestBuildArtifactRequests(t *testing.T) { } }) - t.Run("missing CEL variable", func(t *testing.T) { + t.Run("unknown capture placeholder", func(t *testing.T) { g := gomega.NewWithT(t) obj := &swapi.ArtifactGenerator{ Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ - {Name: "missing"}, + {Name: "{missing}"}, }, }, } - _, err := buildArtifactRequests(ctx, obj, localSources) + _, err := buildArtifactRequests(obj, localSources) g.Expect(err).To(gomega.HaveOccurred()) - g.Expect(err.Error()).To(gomega.ContainSubstring("failed to evaluate the CEL expression")) - g.Expect(err.Error()).To(gomega.ContainSubstring("no such attribute")) + g.Expect(err.Error()).To(gomega.ContainSubstring("capture variable \"missing\" is not defined by pathPattern")) }) t.Run("preserves Exclude and Strategy fields", func(t *testing.T) { @@ -216,11 +213,11 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ { - Name: "'app-' + app", + Name: "app-{app}", Copy: []swapi.CopyOperation{ { - From: "'apps/' + app", - To: "'.'", + From: "apps/{app}", + To: ".", Exclude: []string{"*.md", "tests/"}, Strategy: "Merge", }, @@ -229,7 +226,7 @@ func TestBuildArtifactRequests(t *testing.T) { }, }, } - reqs, err := buildArtifactRequests(ctx, obj, localSources) + reqs, err := buildArtifactRequests(obj, localSources) g.Expect(err).ToNot(gomega.HaveOccurred()) g.Expect(reqs).To(gomega.HaveLen(3)) @@ -245,11 +242,11 @@ func TestBuildArtifactRequests(t *testing.T) { Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}/envs/{env}", OutputArtifacts: []swapi.OutputArtifact{ - {Name: "app + '-' + env"}, + {Name: "{app}-{env}"}, }, }, } - reqs, err := buildArtifactRequests(ctx, obj, localSources) + reqs, err := buildArtifactRequests(obj, localSources) g.Expect(err).ToNot(gomega.HaveOccurred()) g.Expect(reqs).To(gomega.HaveLen(2)) // only auth/envs/dev and auth/envs/prod, ignore is a file @@ -280,15 +277,15 @@ func TestBuildArtifactRequests(t *testing.T) { PathPattern: "@repo/apps/{app}/envs/{env}", OutputArtifacts: []swapi.OutputArtifact{ { - Name: "app + '-' + env", + Name: "{app}-{env}", Copy: []swapi.CopyOperation{ - {From: "'@repo/apps/' + app + '/envs/' + env + '/**'", To: "'@artifact/' + app + '/' + env + '/'"}, + {From: "@repo/apps/{app}/envs/{env}/**", To: "@artifact/{app}/{env}/"}, }, }, }, }, } - reqs, err := buildArtifactRequests(ctx, obj, upperSources) + reqs, err := buildArtifactRequests(obj, upperSources) g.Expect(err).ToNot(gomega.HaveOccurred()) g.Expect(reqs).To(gomega.HaveLen(2)) @@ -329,11 +326,11 @@ func TestBuildArtifactRequests(t *testing.T) { Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ - {Name: "'app-' + app"}, + {Name: "app-{app}"}, }, }, } - _, err = buildArtifactRequests(ctx, obj, dotSources) + _, err = buildArtifactRequests(obj, dotSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes label value")) @@ -355,11 +352,11 @@ func TestBuildArtifactRequests(t *testing.T) { Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ - {Name: "'app-' + app"}, + {Name: "app-{app}"}, }, }, } - _, err = buildArtifactRequests(ctx, obj, spaceSources) + _, err = buildArtifactRequests(obj, spaceSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes label value")) @@ -382,11 +379,11 @@ func TestBuildArtifactRequests(t *testing.T) { Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ - {Name: "'app-' + app"}, + {Name: "app-{app}"}, }, }, } - _, err = buildArtifactRequests(ctx, obj, longSources) + _, err = buildArtifactRequests(obj, longSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes label value")) @@ -408,12 +405,12 @@ func TestBuildArtifactRequests(t *testing.T) { Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ - // CEL expression produces "auth..invalid" which is not a valid DNS subdomain. - {Name: "app + '..invalid'"}, + // Template produces "auth..invalid" which is not a valid DNS subdomain. + {Name: "{app}..invalid"}, }, }, } - _, err = buildArtifactRequests(ctx, obj, dnsSources) + _, err = buildArtifactRequests(obj, dnsSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring("not a valid Kubernetes object name")) @@ -437,11 +434,11 @@ func TestBuildArtifactRequests(t *testing.T) { Spec: swapi.ArtifactGeneratorSpec{ PathPattern: "@repo/apps/{app}", OutputArtifacts: []swapi.OutputArtifact{ - {Name: "'app-' + app"}, + {Name: "app-{app}"}, }, }, } - _, err = buildArtifactRequests(ctx, obj, dupSources) + _, err = buildArtifactRequests(obj, dupSources) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("pathPattern")) g.Expect(err.Error()).To(gomega.ContainSubstring("both resolve to artifact name")) diff --git a/internal/controller/artifactgenerator_validation_test.go b/internal/controller/artifactgenerator_validation_test.go index 157b907d..7d8cfde3 100644 --- a/internal/controller/artifactgenerator_validation_test.go +++ b/internal/controller/artifactgenerator_validation_test.go @@ -248,7 +248,7 @@ func TestArtifactGenerator_crdValidation(t *testing.T) { expectError: true, }, { - name: "CEL artifact name with pathPattern", + name: "templated artifact name with pathPattern", setupObj: func() *swapi.ArtifactGenerator { objKey := client.ObjectKey{ Name: "test-path-pattern-template-name", @@ -257,9 +257,9 @@ func TestArtifactGenerator_crdValidation(t *testing.T) { obj := getArtifactGenerator(objKey) alias := obj.Spec.Sources[0].Alias obj.Spec.PathPattern = "@" + alias + "/apps/{app}" - obj.Spec.OutputArtifacts[0].Name = "app" - obj.Spec.OutputArtifacts[0].Copy[0].From = "'@" + alias + "/apps/' + app + '/**'" - obj.Spec.OutputArtifacts[0].Copy[0].To = "'@artifact/file.yaml'" + obj.Spec.OutputArtifacts[0].Name = "{app}" + obj.Spec.OutputArtifacts[0].Copy[0].From = "@" + alias + "/apps/{app}/**" + obj.Spec.OutputArtifacts[0].Copy[0].To = "@artifact/file.yaml" return obj }, expectError: false,