[Prototyping] Consolidate file ignore logic in parser (#98048)

Consolidate file ignore function in parser

pkg/apis/provisioning/v0alpha1/jobs.go

@@ -1,8 +1,6 @@
 package v0alpha1
 
 import (
-	"path/filepath"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -78,17 +76,6 @@ type JobStatus struct {
 	Errors   []string `json:"errors,omitempty"`
 }
 
-// Filter ignorable files
-type IgnoreFile = func(path string) bool
-
-func IncludeYamlOrJSON(p string) bool { // put this somewhere better
-	ext := filepath.Ext(p)
-	if ext == ".yaml" || ext == ".json" {
-		return false
-	}
-	return true
-}
-
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 type WebhookResponse struct {
 	metav1.TypeMeta `json:",inline"`

pkg/registry/apis/provisioning/jobs/pullrequest.go

@@ -141,9 +141,11 @@ func (c *PullRequestCommenter) Process(ctx context.Context, job provisioning.Job
 	previews := make([]resourcePreview, 0, len(files))
 
 	for _, f := range files {
-		// TODO: ignore files?
-		logger := logger.With("file", f.Path)
+		if c.parser.ShouldIgnore(ctx, logger, f.Path) {
+			continue
+		}
 
+		logger := logger.With("file", f.Path)
 		fileInfo, err := c.repo.Read(ctx, logger, f.Path, ref)
 		if err != nil {
 			return fmt.Errorf("failed to read file %s: %w", f.Path, err)
@@ -175,7 +177,7 @@ func (c *PullRequestCommenter) Process(ctx context.Context, job provisioning.Job
 		preview := resourcePreview{
 			Filename: path.Base(f.Path),
 			Path:     f.Path,
-			Kind:     "dashboard", // TODO: add more types
+			Kind:     "dashboard", // TODO: add more kinds
 			Action:   string(f.Action),
 		}
 

pkg/registry/apis/provisioning/jobs/worker.go

@@ -28,7 +28,6 @@ type JobWorker struct {
 	parsers     *resources.ParserFactory
 	identities  auth.BackgroundIdentityService
 	logger      *slog.Logger
-	ignore      provisioning.IgnoreFile
 	render      rendering.Service
 	blobstore   blob.PublicBlobStore
 	urlProvider func(namespace string) string
@@ -39,7 +38,6 @@ func NewJobWorker(
 	parsers *resources.ParserFactory,
 	identities auth.BackgroundIdentityService,
 	logger *slog.Logger,
-	ignore provisioning.IgnoreFile,
 	render rendering.Service,
 	blobstore blob.PublicBlobStore,
 	urlProvider func(namespace string) string,
@@ -49,7 +47,6 @@ func NewJobWorker(
 		parsers:     parsers,
 		identities:  identities,
 		logger:      logger,
-		ignore:      ignore,
 		render:      render,
 		blobstore:   blobstore,
 		urlProvider: urlProvider,
@@ -83,7 +80,7 @@ func (g *JobWorker) Process(ctx context.Context, job provisioning.Job) (*provisi
 		return nil, fmt.Errorf("failed to get parser for %s: %w", repo.Config().Name, err)
 	}
 
-	replicator, err := resources.NewReplicator(repo, parser, g.ignore, logger)
+	replicator, err := resources.NewReplicator(repo, parser, logger)
 	if err != nil {
 		return nil, fmt.Errorf("error creating replicator")
 	}

pkg/registry/apis/provisioning/register.go

@@ -186,7 +186,6 @@ func (b *ProvisioningAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserv
 		b.parsers,
 		b.identities,
 		b.logger.With("worker", "github"),
-		provisioning.IncludeYamlOrJSON,
 		b.render,
 		b.blobstore,
 		b.urlProvider,

pkg/registry/apis/provisioning/repository/github.go

@@ -23,7 +23,6 @@ type githubRepository struct {
 	logger *slog.Logger
 	config *provisioning.Repository
 	gh     pgh.Client
-	ignore provisioning.IgnoreFile
 }
 
 var _ Repository = (*githubRepository)(nil)
@@ -37,7 +36,6 @@ func NewGitHub(
 		config: config,
 		logger: slog.Default().With("logger", "github-repository"),
 		gh:     factory.New(ctx, config.Spec.GitHub.Token),
-		ignore: provisioning.IncludeYamlOrJSON,
 	}
 }
 
@@ -460,37 +458,6 @@ func (r *githubRepository) parsePushEvent(event *github.PushEvent) (*provisionin
 		return &provisioning.WebhookResponse{Code: http.StatusOK}, nil
 	}
 
-	var count int
-	for _, commit := range event.Commits {
-		for _, file := range commit.Added {
-			if r.ignore(file) {
-				continue
-			}
-			count++
-		}
-
-		for _, file := range commit.Modified {
-			if r.ignore(file) {
-				continue
-			}
-			count++
-		}
-
-		for _, file := range commit.Removed {
-			if r.ignore(file) {
-				continue
-			}
-			count++
-		}
-	}
-
-	if count == 0 {
-		return &provisioning.WebhookResponse{
-			Code:    http.StatusOK, // Nothing needed
-			Message: "no files require updates",
-		}, nil
-	}
-
 	return &provisioning.WebhookResponse{
 		Code: http.StatusAccepted,
 		Job: &provisioning.JobSpec{
@@ -579,10 +546,6 @@ func (r *githubRepository) CompareFiles(ctx context.Context, logger *slog.Logger
 
 	changes := make([]FileChange, 0)
 	for _, f := range files {
-		// TODO: where should I ignore files?
-		if r.ignore(f.GetFilename()) {
-			continue
-		}
 		// reference: https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#get-a-commit
 		switch f.GetStatus() {
 		case "added", "copied":

pkg/registry/apis/provisioning/repository/github_test.go

@@ -101,7 +101,6 @@ func TestParseWebhooks(t *testing.T) {
 			},
 		},
 		logger: slog.Default().With("logger", "github-repository"),
-		ignore: provisioning.IncludeYamlOrJSON,
 	}
 
 	for _, tt := range tests {

pkg/registry/apis/provisioning/resources/parser.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"log/slog"
 	"path"
+	"path/filepath"
 
 	"gopkg.in/yaml.v3"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -122,11 +123,24 @@ func (r *Parser) Client() *DynamicClient {
 	return r.client
 }
 
+func (r *Parser) ShouldIgnore(ctx context.Context, logger *slog.Logger, p string) bool {
+	ext := filepath.Ext(p)
+	if ext == ".yaml" || ext == ".json" {
+		return false
+	}
+
+	return true
+}
+
 func (r *Parser) Parse(ctx context.Context, logger *slog.Logger, info *repository.FileInfo, validate bool) (parsed *ParsedResource, err error) {
 	parsed = &ParsedResource{
 		Info: info,
 	}
 
+	if r.ShouldIgnore(ctx, logger, info.Path) {
+		return parsed, ErrUnableToReadResourceBytes
+	}
+
 	parsed.Obj, parsed.GVK, err = LoadYAMLOrJSON(bytes.NewBuffer(info.Data))
 	if err != nil {
 		logger.DebugContext(ctx, "failed to find GVK of the input data", "error", err)

pkg/registry/apis/provisioning/resources/replicator.go

@@ -28,13 +28,11 @@ type Replicator struct {
 	parser     *Parser
 	folders    dynamic.ResourceInterface
 	repository repository.Repository
-	ignore     provisioning.IgnoreFile
 }
 
 func NewReplicator(
 	repo repository.Repository,
 	parser *Parser,
-	ignore provisioning.IgnoreFile,
 	logger *slog.Logger,
 ) (*Replicator, error) {
 	dynamicClient := parser.Client()
@@ -50,7 +48,6 @@ func NewReplicator(
 		client:     dynamicClient,
 		folders:    folders,
 		repository: repo,
-		ignore:     ignore,
 	}, nil
 }
 
@@ -121,7 +118,7 @@ func (r *Replicator) replicateTree(ctx context.Context, ref string) error {
 			continue
 		}
 
-		if r.ignore(entry.Path) {
+		if r.parser.ShouldIgnore(ctx, logger, entry.Path) {
 			logger.DebugContext(ctx, "ignoring file")
 			continue
 		}
@@ -236,6 +233,10 @@ func (r *Replicator) createFolderPath(ctx context.Context, filePath string) (str
 
 func (r *Replicator) replicateChanges(ctx context.Context, changes []repository.FileChange) error {
 	for _, change := range changes {
+		if r.parser.ShouldIgnore(ctx, r.logger, change.Path) {
+			continue
+		}
+
 		fileInfo, err := r.repository.Read(ctx, r.logger, change.Path, change.Ref)
 		if err != nil {
 			return fmt.Errorf("read file: %w", err)