diff --git a/api/v1beta1/artifactgenerator_types.go b/api/v1beta1/artifactgenerator_types.go index 0a9f65a..ea28432 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) && 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 @@ -68,6 +69,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 placeholders 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 +119,8 @@ 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])?$" + // When pathPattern is set, this field may use capture placeholders such as "{app}". + // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=253 // +required Name string `json:"name"` @@ -144,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 "@/". + // The format is "@/". When pathPattern is set, + // 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 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. + // refers to the root path of the generated artifact. When pathPattern + // is set, the path may use capture placeholders such as "{app}". // +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 9279168..a131dbe 100644 --- a/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml +++ b/config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml @@ -79,8 +79,10 @@ 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, + the path may use capture placeholders such as "{app}". maxLength: 1024 + minLength: 1 pattern: ^@([a-z0-9]([a-z0-9_-]*[a-z0-9])?)/(.*)$ type: string strategy: @@ -101,8 +103,10 @@ 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, the path may use capture placeholders such as "{app}". maxLength: 1024 + minLength: 1 pattern: ^@(artifact)/(.*)$ type: string required: @@ -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 may use capture placeholders such as "{app}". maxLength: 253 - pattern: ^[a-z0-9]([a-z0-9-]*[a-z0-9])?$ + minLength: 1 type: string originRevision: description: |- @@ -159,6 +165,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 placeholders 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 @@ -210,6 +224,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) && 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. properties: diff --git a/docs/spec/v1beta1/artifactgenerators.md b/docs/spec/v1beta1/artifactgenerators.md index 3ed1474..dc5992e 100644 --- a/docs/spec/v1beta1/artifactgenerators.md +++ b/docs/spec/v1beta1/artifactgenerators.md @@ -227,13 +227,60 @@ 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 placeholders in 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 captured values before using them +in artifact names and labels. This means a directory named `Auth` will produce +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 +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 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. + 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. @@ -262,10 +309,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 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 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/internal/controller/artifactgenerator_controller.go b/internal/controller/artifactgenerator_controller.go index 415a83c..dc6c140 100644 --- a/internal/controller/artifactgenerator_controller.go +++ b/internal/controller/artifactgenerator_controller.go @@ -193,14 +193,33 @@ 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()) + 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, + "%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 +238,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 +442,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 +458,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 2829983..3c5fe23 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" ) @@ -529,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("%s-{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() @@ -848,3 +907,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.go b/internal/controller/artifactgenerator_drift.go index 9120e58..02cbbb7 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 e2a2eea..8f740b1 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 { @@ -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 { diff --git a/internal/controller/artifactgenerator_finalize_test.go b/internal/controller/artifactgenerator_finalize_test.go index a58f3d0..9dc577c 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_pathpattern.go b/internal/controller/artifactgenerator_pathpattern.go new file mode 100644 index 0000000..fe3f1bf --- /dev/null +++ b/internal/controller/artifactgenerator_pathpattern.go @@ -0,0 +1,296 @@ +/* +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 ( + "errors" + "fmt" + "io/fs" + "path/filepath" + "regexp" + "strings" + + 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 +} + +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(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, newTerminalPathPatternError("invalid pathPattern format: %s", obj.Spec.PathPattern) + } + + alias := strings.TrimPrefix(parts[0], "@") + patternStr := parts[1] + + srcDir, ok := localSources[alias] + if !ok { + 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, 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, 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, newTerminalPathPatternError( + "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. + 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. + rawCaptures := make(map[string]string) + for i, name := range subexpNames { + if i != 0 && name != "" { + rawCaptures[name] = matches[i] + } + } + + // 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 normalizedCaptures { + if errs := content.IsLabelValue(v); len(errs) > 0 { + 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, "; ")) + } + } + + // Render the OutputArtifacts using the captured variables. + for _, oa := range obj.Spec.OutputArtifacts { + 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) + } + + // Validate rendered EA name with NameIsDNSSubdomain. + if errs := apivalidation.NameIsDNSSubdomain(req.Name, false); len(errs) > 0 { + 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 newTerminalPathPatternError( + "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 { + if isTerminalPathPatternError(err) { + return nil, err + } + return nil, fmt.Errorf("failed to walk source directory: %w", err) + } + + 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 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) + } + 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 substituting capture placeholders. +// 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 := renderTemplateString(oa.Name, normalizedCaptures) + if err != nil { + return artifactRequest{}, err + } + + req := artifactRequest{ + OutputArtifact: oa, + Labels: normalizedCaptures, + } + req.Name = name + + // Deep copy the Copy slice to avoid mutating the original OutputArtifact spec, + // and substitute capture placeholders in each copy operation. + req.Copy = make([]swapi.CopyOperation, len(oa.Copy)) + for i, copyOp := range oa.Copy { + from, err := renderTemplateString(copyOp.From, rawCaptures) + if err != nil { + return artifactRequest{}, err + } + + to, err := renderTemplateString(copyOp.To, rawCaptures) + if err != nil { + return artifactRequest{}, err + } + + req.Copy[i] = swapi.CopyOperation{ + From: from, + To: to, + Exclude: copyOp.Exclude, + Strategy: copyOp.Strategy, + } + } + + return req, nil +} + +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]) + } + } + return b.String(), nil +} diff --git a/internal/controller/artifactgenerator_pathpattern_test.go b/internal/controller/artifactgenerator_pathpattern_test.go new file mode 100644 index 0000000..ce1e3b5 --- /dev/null +++ b/internal/controller/artifactgenerator_pathpattern_test.go @@ -0,0 +1,446 @@ +/* +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("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(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(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 + pattern string + wantErrMsg string + }{ + {name: "empty capture", pattern: "@repo/apps/{}", wantErrMsg: "empty capture variable"}, + {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"}, + } + + 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{ + 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("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}"}, + }, + }, + } + _, err := buildArtifactRequests(obj, localSources) + g.Expect(err).To(gomega.HaveOccurred()) + 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) { + 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/{app}/{env}/"}, + }, + }, + }, + }, + } + 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))) + } + + 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/")) + } + + } + }) + + 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")) + }) +} diff --git a/internal/controller/artifactgenerator_validation.go b/internal/controller/artifactgenerator_validation.go index eb55371..76e020d 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 927cb43..7d8cfde 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) @@ -234,6 +234,36 @@ 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}/**" + obj.Spec.OutputArtifacts[0].Copy[0].To = "@artifact/file.yaml" + return obj + }, + expectError: false, + }, { name: "invalid artifact copy from", setupObj: func() *swapi.ArtifactGenerator {