diff --git a/porch/engine/pkg/engine/engine.go b/porch/engine/pkg/engine/engine.go index e80341c155..bb28f4da38 100644 --- a/porch/engine/pkg/engine/engine.go +++ b/porch/engine/pkg/engine/engine.go @@ -64,6 +64,9 @@ type cadEngine struct { var _ CaDEngine = &cadEngine{} type mutation interface { + // Apply applies the mutation to the specified resources. + // It returns the updated resources, along with the task as it should be represented in the PackageRevision spec.tasks. + // A mutation can return a nil task to indicate that we should not record a task in the PackageRevision spec.tasks. Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.Task, error) } diff --git a/porch/engine/pkg/engine/render.go b/porch/engine/pkg/engine/render.go index 3f42055e74..5e696a7bca 100644 --- a/porch/engine/pkg/engine/render.go +++ b/porch/engine/pkg/engine/render.go @@ -60,14 +60,8 @@ func (m *renderPackageMutation) Apply(ctx context.Context, resources repository. return repository.PackageResources{}, nil, err } - // TODO: There are internal tasks not represented in the API; Update the Apply interface to enable them. - return result, &api.Task{ - Type: "eval", - Eval: &api.FunctionEvalTaskSpec{ - Image: "render", - ConfigMap: map[string]string{}, - }, - }, nil + // This is an internal task not represented in the API, we return nil to represent this. + return result, nil, nil } // TODO: Implement filesystem abstraction directly rather than on top of PackageResources diff --git a/porch/repository/pkg/git/draft.go b/porch/repository/pkg/git/draft.go index b93e9f2b4c..1a75a1b562 100644 --- a/porch/repository/pkg/git/draft.go +++ b/porch/repository/pkg/git/draft.go @@ -41,7 +41,7 @@ type gitPackageDraft struct { var _ repository.PackageDraft = &gitPackageDraft{} -func (d *gitPackageDraft) UpdateResources(ctx context.Context, new *v1alpha1.PackageRevisionResources, change *v1alpha1.Task) error { +func (d *gitPackageDraft) UpdateResources(ctx context.Context, new *v1alpha1.PackageRevisionResources, task *v1alpha1.Task) error { ch, err := newCommitHelper(d.parent.repo.Storer, d.parent.userInfoProvider, d.commit, d.path, plumbing.ZeroHash) if err != nil { return fmt.Errorf("failed to commit packgae: %w", err) @@ -51,7 +51,13 @@ func (d *gitPackageDraft) UpdateResources(ctx context.Context, new *v1alpha1.Pac ch.storeFile(path.Join(d.path, k), v) } - message := fmt.Sprintf("Intermittent commit: %s", change.Type) + var message string + if task != nil { + message = fmt.Sprintf("Intermittent commit: %s", task.Type) + } else { + // TODO: Safe to assume it's always a render? + message = "Internal commit (render)" + } commitHash, packageTree, err := ch.commit(ctx, message, d.path) if err != nil { return fmt.Errorf("failed to commit package: %w", err) diff --git a/porch/repository/pkg/oci/mutate.go b/porch/repository/pkg/oci/mutate.go index 2ae4f8da2c..0d0d6639ae 100644 --- a/porch/repository/pkg/oci/mutate.go +++ b/porch/repository/pkg/oci/mutate.go @@ -152,9 +152,20 @@ func (p *ociPackageDraft) UpdateResources(ctx context.Context, new *api.PackageR return fmt.Errorf("failed to write remote layer: %w", err) } - taskJSON, err := json.Marshal(*task) - if err != nil { - return fmt.Errorf("failed to marshal task %T to json: %w", task, err) + history := v1.History{ + Author: "kool kat", + Created: v1.Time{Time: p.created}, + } + + if task != nil { + taskJSON, err := json.Marshal(*task) + if err != nil { + return fmt.Errorf("failed to marshal task %T to json: %w", task, err) + } + + history.CreatedBy = "kpt:" + string(taskJSON) + } else { + // TODO: Mark as internal in some way? } digest, err := layer.Digest() @@ -170,15 +181,12 @@ func (p *ociPackageDraft) UpdateResources(ctx context.Context, new *api.PackageR } p.addendums = append(p.addendums, mutate.Addendum{ - Layer: remoteLayer, - History: v1.History{ - Author: "kool kat", - Created: v1.Time{Time: p.created}, - CreatedBy: "kpt:" + string(taskJSON), - }, + Layer: remoteLayer, + History: history, }) - - p.tasks = append(p.tasks, *task) + if task != nil { + p.tasks = append(p.tasks, *task) + } return nil }