From eb53a449d5b2d7455bcb51a2db24eb065312f332 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Fri, 14 Oct 2022 12:42:40 -0400 Subject: [PATCH] Populate package-path metadata into package context We construct a path based on the hierarchy of parent packages. This is useful for packages/package functions that want to construct values meaningful beyond just their package - for example domain names, GCP project IDs, bucket names, descriptions etc. --- internal/builtins/pkg_context.go | 39 +++-- porch/pkg/engine/builtin.go | 12 +- porch/pkg/engine/builtin_test.go | 15 +- porch/pkg/engine/clone.go | 9 +- porch/pkg/engine/engine.go | 145 ++++++++++++++++-- .../context/expected/package-context.yaml | 1 + porch/pkg/registry/porch/packagecommon.go | 13 +- porch/pkg/registry/porch/packagerevision.go | 13 +- 8 files changed, 201 insertions(+), 46 deletions(-) diff --git a/internal/builtins/pkg_context.go b/internal/builtins/pkg_context.go index 614497aceb..fc1a7bc12a 100644 --- a/internal/builtins/pkg_context.go +++ b/internal/builtins/pkg_context.go @@ -30,7 +30,9 @@ import ( const ( PkgContextFile = "package-context.yaml" - pkgContextName = "kptfile.kpt.dev" + PkgContextName = "kptfile.kpt.dev" + + ConfigKeyPackagePath = "package-path" ) var ( @@ -42,7 +44,17 @@ var ( // a KRM object that contains package context information that can be // used by functions such as `set-namespace` to customize package with // minimal configuration. -type PackageContextGenerator struct{} +type PackageContextGenerator struct { + // PackageConfig contains the package configuration to set. + PackageConfig *PackageConfig +} + +// PackageConfig holds package automatic configuration +type PackageConfig struct { + // PackagePath is the path to the package, as determined by the names of the parent packages. + // The path to a package is the parent package path joined with the package name. + PackagePath string +} // Run function reads the function input `resourceList` from a given reader `r` // and writes the function output to the provided writer `w`. @@ -66,14 +78,14 @@ func (pc *PackageContextGenerator) Process(resourceList *framework.ResourceList) // - Generates a package context resource for each kpt package (i.e Kptfile) for _, resource := range resourceList.Items { gvk := resid.GvkFromNode(resource) - if gvk.Equals(configMapGVK) && resource.GetName() == pkgContextName { + if gvk.Equals(configMapGVK) && resource.GetName() == PkgContextName { // drop existing package context resources continue } updatedResources = append(updatedResources, resource) if gvk.Equals(kptfileGVK) { // it's a Kptfile, generate a corresponding package context - pkgContext, err := pkgContextResource(resource) + pkgContext, err := pkgContextResource(resource, pc.PackageConfig) if err != nil { resourceList.Results = framework.Results{ &framework.Result{ @@ -102,10 +114,10 @@ func (pc *PackageContextGenerator) Process(resourceList *framework.ResourceList) // pkgContextResource generates package context resource from a given // Kptfile. The resource is generated adjacent to the Kptfile of the package. -func pkgContextResource(kf *yaml.RNode) (*yaml.RNode, error) { +func pkgContextResource(kptfile *yaml.RNode, packageConfig *PackageConfig) (*yaml.RNode, error) { cm := yaml.MustParse(AbstractPkgContext()) - kptfilePath, _, err := kioutil.GetFileAnnotations(kf) + kptfilePath, _, err := kioutil.GetFileAnnotations(kptfile) if err != nil { return nil, err } @@ -118,9 +130,16 @@ func pkgContextResource(kf *yaml.RNode) (*yaml.RNode, error) { return nil, err } } - cm.SetDataMap(map[string]string{ - "name": kf.GetName(), - }) + data := map[string]string{ + "name": kptfile.GetName(), + } + if packageConfig != nil { + if packageConfig.PackagePath != "" { + data[ConfigKeyPackagePath] = packageConfig.PackagePath + } + } + + cm.SetDataMap(data) return cm, nil } @@ -136,5 +155,5 @@ metadata: config.kubernetes.io/local-config: "true" data: name: example -`, pkgContextName) +`, PkgContextName) } diff --git a/porch/pkg/engine/builtin.go b/porch/pkg/engine/builtin.go index eb97fe0b12..abf5299f0c 100644 --- a/porch/pkg/engine/builtin.go +++ b/porch/pkg/engine/builtin.go @@ -33,17 +33,13 @@ type builtinEvalMutation struct { runner fn.FunctionRunner } -func newBuiltinFunctionMutation(function string) (mutation, error) { - var runner fn.FunctionRunner - switch function { - case fnruntime.FuncGenPkgContext: - runner = &builtins.PackageContextGenerator{} - default: - return nil, fmt.Errorf("unrecognized built-in function %q", function) +func newPackageContextGeneratorMutation(packageConfig *builtins.PackageConfig) (mutation, error) { + runner := &builtins.PackageContextGenerator{ + PackageConfig: packageConfig, } return &builtinEvalMutation{ - function: function, + function: fnruntime.FuncGenPkgContext, runner: runner, }, nil } diff --git a/porch/pkg/engine/builtin_test.go b/porch/pkg/engine/builtin_test.go index bc4bf5091b..6a770bf47b 100644 --- a/porch/pkg/engine/builtin_test.go +++ b/porch/pkg/engine/builtin_test.go @@ -20,7 +20,7 @@ import ( "path/filepath" "testing" - "github.com/GoogleContainerTools/kpt/internal/fnruntime" + "github.com/GoogleContainerTools/kpt/internal/builtins" "github.com/google/go-cmp/cmp" ) @@ -28,14 +28,6 @@ const ( updateGoldenFiles = "UPDATE_GOLDEN_FILES" ) -func TestUnknownBuiltinFunctionMutation(t *testing.T) { - const doesNotExist = "function-does-not-exist" - m, err := newBuiltinFunctionMutation(doesNotExist) - if err == nil || m != nil { - t.Errorf("creating builtin function runner for an unknown function %q unexpectedly succeeded", doesNotExist) - } -} - func TestPackageContext(t *testing.T) { testdata, err := filepath.Abs(filepath.Join(".", "testdata", "context")) if err != nil { @@ -44,7 +36,10 @@ func TestPackageContext(t *testing.T) { input := readPackage(t, filepath.Join(testdata, "input")) - m, err := newBuiltinFunctionMutation(fnruntime.FuncGenPkgContext) + packageConfig := &builtins.PackageConfig{ + PackagePath: "parent1/parent1.2/parent1.2.3/me", + } + m, err := newPackageContextGeneratorMutation(packageConfig) if err != nil { t.Fatalf("Failed to get builtin function mutation: %v", err) } diff --git a/porch/pkg/engine/clone.go b/porch/pkg/engine/clone.go index fa53f2dd03..92d58f21df 100644 --- a/porch/pkg/engine/clone.go +++ b/porch/pkg/engine/clone.go @@ -21,7 +21,7 @@ import ( "os" "strings" - "github.com/GoogleContainerTools/kpt/internal/fnruntime" + "github.com/GoogleContainerTools/kpt/internal/builtins" v1 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1" api "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" configapi "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1" @@ -44,6 +44,9 @@ type clonePackageMutation struct { repoOpener RepositoryOpener credentialResolver repository.CredentialResolver referenceResolver ReferenceResolver + + // packageConfig contains the package configuration. + packageConfig *builtins.PackageConfig } func (m *clonePackageMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.Task, error) { @@ -77,13 +80,13 @@ func (m *clonePackageMutation) Apply(ctx context.Context, resources repository.P if m.isDeployment { // TODO(droot): executing this as mutation is not really needed, but can be // refactored once we finalize the task/mutation/commit model. - genPkgContextMutation, err := newBuiltinFunctionMutation(fnruntime.FuncGenPkgContext) + genPkgContextMutation, err := newPackageContextGeneratorMutation(m.packageConfig) if err != nil { return repository.PackageResources{}, nil, err } cloned, _, err = genPkgContextMutation.Apply(ctx, cloned) if err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("failed to generate deployment context %w", err) + return repository.PackageResources{}, nil, fmt.Errorf("failed to generate deployment context: %w", err) } } diff --git a/porch/pkg/engine/engine.go b/porch/pkg/engine/engine.go index 657ffd4c30..c25277f45e 100644 --- a/porch/pkg/engine/engine.go +++ b/porch/pkg/engine/engine.go @@ -20,9 +20,13 @@ import ( "fmt" "io/fs" "os" + "path" "path/filepath" "reflect" + "strings" + "unicode" + "github.com/GoogleContainerTools/kpt/internal/builtins" kptfile "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1" "github.com/GoogleContainerTools/kpt/pkg/fn" api "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" @@ -33,7 +37,11 @@ import ( "github.com/GoogleContainerTools/kpt/porch/pkg/repository" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/trace" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2" "sigs.k8s.io/kustomize/kyaml/comments" @@ -51,8 +59,8 @@ type CaDEngine interface { ListFunctions(ctx context.Context, repositoryObj *configapi.Repository) ([]*Function, error) ListPackageRevisions(ctx context.Context, repositorySpec *configapi.Repository, filter repository.ListPackageRevisionFilter) ([]*PackageRevision, error) - CreatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, obj *api.PackageRevision) (*PackageRevision, error) - UpdatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, old, new *api.PackageRevision) (*PackageRevision, error) + CreatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, obj *api.PackageRevision, parent *PackageRevision) (*PackageRevision, error) + UpdatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, old, new *api.PackageRevision, parent *PackageRevision) (*PackageRevision, error) DeletePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, obj *PackageRevision) error ListPackages(ctx context.Context, repositorySpec *configapi.Repository, filter repository.ListPackageFilter) ([]*Package, error) @@ -191,10 +199,59 @@ func (cad *cadEngine) ListPackageRevisions(ctx context.Context, repositorySpec * return packageRevisions, nil } -func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, obj *api.PackageRevision) (*PackageRevision, error) { +func buildPackageConfig(ctx context.Context, obj *api.PackageRevision, parent *PackageRevision) (*builtins.PackageConfig, error) { + config := &builtins.PackageConfig{} + + parentPath := "" + + var parentConfig *unstructured.Unstructured + if parent != nil { + parentObj, err := parent.GetPackageRevision(ctx) + if err != nil { + return nil, err + } + parentPath = parentObj.Spec.PackageName + + resources, err := parent.GetResources(ctx) + if err != nil { + return nil, fmt.Errorf("error getting resources from parent package %q: %w", parentObj.Name, err) + } + configMapObj, err := ExtractContextConfigMap(resources.Spec.Resources) + if err != nil { + return nil, fmt.Errorf("error getting configuration from parent package %q: %w", parentObj.Name, err) + } + parentConfig = configMapObj + + if parentConfig != nil { + // TODO: Should we support kinds other than configmaps? + var parentConfigMap corev1.ConfigMap + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(parentConfig.Object, &parentConfigMap); err != nil { + return nil, fmt.Errorf("error parsing ConfigMap from parent configuration: %w", err) + } + if s := parentConfigMap.Data[builtins.ConfigKeyPackagePath]; s != "" { + parentPath = s + "/" + parentPath + } + } + } + + if parentPath == "" { + config.PackagePath = obj.Spec.PackageName + } else { + config.PackagePath = parentPath + "/" + obj.Spec.PackageName + } + + return config, nil +} + +func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, obj *api.PackageRevision, parent *PackageRevision) (*PackageRevision, error) { ctx, span := tracer.Start(ctx, "cadEngine::CreatePackageRevision", trace.WithAttributes()) defer span.End() + packageConfig, err := buildPackageConfig(ctx, obj, parent) + if err != nil { + return nil, err + } + // Validate package lifecycle. Cannot create a final package switch obj.Spec.Lifecycle { case "": @@ -218,7 +275,7 @@ func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj * return nil, err } - if err := cad.applyTasks(ctx, draft, repositoryObj, obj); err != nil { + if err := cad.applyTasks(ctx, draft, repositoryObj, obj, packageConfig); err != nil { return nil, err } @@ -247,7 +304,7 @@ func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj * }, nil } -func (cad *cadEngine) applyTasks(ctx context.Context, draft repository.PackageDraft, repositoryObj *configapi.Repository, obj *api.PackageRevision) error { +func (cad *cadEngine) applyTasks(ctx context.Context, draft repository.PackageDraft, repositoryObj *configapi.Repository, obj *api.PackageRevision, packageConfig *builtins.PackageConfig) error { var mutations []mutation // Unless first task is Init or Clone, insert Init to create an empty package. @@ -266,7 +323,7 @@ func (cad *cadEngine) applyTasks(ctx context.Context, draft repository.PackageDr for i := range tasks { task := &tasks[i] - mutation, err := cad.mapTaskToMutation(ctx, obj, task, repositoryObj.Spec.Deployment) + mutation, err := cad.mapTaskToMutation(ctx, obj, task, repositoryObj.Spec.Deployment, packageConfig) if err != nil { return err } @@ -288,7 +345,7 @@ type RepositoryOpener interface { OpenRepository(ctx context.Context, repositorySpec *configapi.Repository) (repository.Repository, error) } -func (cad *cadEngine) mapTaskToMutation(ctx context.Context, obj *api.PackageRevision, task *api.Task, isDeployment bool) (mutation, error) { +func (cad *cadEngine) mapTaskToMutation(ctx context.Context, obj *api.PackageRevision, task *api.Task, isDeployment bool, packageConfig *builtins.PackageConfig) (mutation, error) { switch task.Type { case api.TaskTypeInit: if task.Init == nil { @@ -310,6 +367,7 @@ func (cad *cadEngine) mapTaskToMutation(ctx context.Context, obj *api.PackageRev repoOpener: cad, credentialResolver: cad.credentialResolver, referenceResolver: cad.referenceResolver, + packageConfig: packageConfig, }, nil case api.TaskTypeUpdate: @@ -366,7 +424,7 @@ func (cad *cadEngine) mapTaskToMutation(ctx context.Context, obj *api.PackageRev } } -func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, oldObj, newObj *api.PackageRevision) (*PackageRevision, error) { +func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, oldObj, newObj *api.PackageRevision, parent *PackageRevision) (*PackageRevision, error) { ctx, span := tracer.Start(ctx, "cadEngine::UpdatePackageRevision", trace.WithAttributes()) defer span.End() @@ -406,7 +464,11 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj * } if isRecloneAndReplay(oldObj, newObj) { - repoPkgRev, err := cad.recloneAndReplay(ctx, repo, repositoryObj, newObj) + packageConfig, err := buildPackageConfig(ctx, newObj, parent) + if err != nil { + return nil, err + } + repoPkgRev, err := cad.recloneAndReplay(ctx, repo, repositoryObj, newObj, packageConfig) if err != nil { return nil, err } @@ -1098,7 +1160,7 @@ func isRecloneAndReplay(oldObj, newObj *api.PackageRevision) bool { // recloneAndReplay performs an update by recloning the upstream package and replaying all tasks. // This is more like a git rebase operation than the "classic" kpt update algorithm, which is more like a git merge. -func (cad *cadEngine) recloneAndReplay(ctx context.Context, repo repository.Repository, repositoryObj *configapi.Repository, newObj *api.PackageRevision) (repository.PackageRevision, error) { +func (cad *cadEngine) recloneAndReplay(ctx context.Context, repo repository.Repository, repositoryObj *configapi.Repository, newObj *api.PackageRevision, packageConfig *builtins.PackageConfig) (repository.PackageRevision, error) { ctx, span := tracer.Start(ctx, "cadEngine::recloneAndReplay", trace.WithAttributes()) defer span.End() @@ -1109,7 +1171,7 @@ func (cad *cadEngine) recloneAndReplay(ctx context.Context, repo repository.Repo return nil, err } - if err := cad.applyTasks(ctx, draft, repositoryObj, newObj); err != nil { + if err := cad.applyTasks(ctx, draft, repositoryObj, newObj, packageConfig); err != nil { return nil, err } @@ -1119,3 +1181,64 @@ func (cad *cadEngine) recloneAndReplay(ctx context.Context, repo repository.Repo return draft.Close(ctx) } + +// ExtractContextConfigMap returns the package-context configmap, if found +func ExtractContextConfigMap(resources map[string]string) (*unstructured.Unstructured, error) { + var matches []*unstructured.Unstructured + + for itemPath, itemContents := range resources { + ext := path.Ext(itemPath) + ext = strings.ToLower(ext) + + parse := false + switch ext { + case ".yaml", ".yml": + parse = true + + default: + klog.Warningf("ignoring non-yaml file %s", itemPath) + } + + if !parse { + continue + } + // TODO: Use https://github.com/kubernetes-sigs/kustomize/blob/a5b61016bb40c30dd1b0a78290b28b2330a0383e/kyaml/kio/byteio_reader.go#L170 or similar? + for _, s := range strings.Split(itemContents, "\n---\n") { + if isWhitespace(s) { + continue + } + + o := &unstructured.Unstructured{} + if err := yaml.Unmarshal([]byte(s), &o); err != nil { + return nil, fmt.Errorf("error parsing yaml from %s: %w", itemPath, err) + } + + // TODO: sync with kpt logic; skip objects marked with the local-only annotation + + configMapGK := schema.GroupKind{Kind: "ConfigMap"} + if o.GroupVersionKind().GroupKind() == configMapGK { + if o.GetName() == builtins.PkgContextName { + matches = append(matches, o) + } + } + } + } + if len(matches) == 0 { + return nil, nil + } + + if len(matches) > 1 { + return nil, fmt.Errorf("found multiple configmaps matching name %q", builtins.PkgContextFile) + } + + return matches[0], nil +} + +func isWhitespace(s string) bool { + for _, r := range s { + if !unicode.IsSpace(r) { + return false + } + } + return true +} diff --git a/porch/pkg/engine/testdata/context/expected/package-context.yaml b/porch/pkg/engine/testdata/context/expected/package-context.yaml index d1b0f5b9db..57b4e523a7 100644 --- a/porch/pkg/engine/testdata/context/expected/package-context.yaml +++ b/porch/pkg/engine/testdata/context/expected/package-context.yaml @@ -6,3 +6,4 @@ metadata: config.kubernetes.io/local-config: "true" data: name: input + package-path: parent1/parent1.2/parent1.2.3/me diff --git a/porch/pkg/registry/porch/packagecommon.go b/porch/pkg/registry/porch/packagecommon.go index 627e166737..a06433e2f1 100644 --- a/porch/pkg/registry/porch/packagecommon.go +++ b/porch/pkg/registry/porch/packagecommon.go @@ -275,8 +275,17 @@ func (r *packageCommon) updatePackageRevision(ctx context.Context, name string, return nil, false, apierrors.NewInternalError(fmt.Errorf("error getting repository %v: %w", repositoryID, err)) } + var parentPackage *engine.PackageRevision + if newApiPkgRev.Spec.Parent != nil && newApiPkgRev.Spec.Parent.Name != "" { + p, err := r.getRepoPkgRev(ctx, newApiPkgRev.Spec.Parent.Name) + if err != nil { + return nil, false, fmt.Errorf("cannot get parent package %q: %w", newApiPkgRev.Spec.Parent.Name, err) + } + parentPackage = p + } + if !isCreate { - rev, err := r.cad.UpdatePackageRevision(ctx, &repositoryObj, oldRepoPkgRev, oldApiPkgRev.(*api.PackageRevision), newApiPkgRev) + rev, err := r.cad.UpdatePackageRevision(ctx, &repositoryObj, oldRepoPkgRev, oldApiPkgRev.(*api.PackageRevision), newApiPkgRev, parentPackage) if err != nil { return nil, false, apierrors.NewInternalError(err) } @@ -288,7 +297,7 @@ func (r *packageCommon) updatePackageRevision(ctx context.Context, name string, return updated, false, nil } else { - rev, err := r.cad.CreatePackageRevision(ctx, &repositoryObj, newApiPkgRev) + rev, err := r.cad.CreatePackageRevision(ctx, &repositoryObj, newApiPkgRev, parentPackage) if err != nil { klog.Infof("error creating package: %v", err) return nil, false, apierrors.NewInternalError(err) diff --git a/porch/pkg/registry/porch/packagerevision.go b/porch/pkg/registry/porch/packagerevision.go index db22c3bba6..eb9dc7d9b4 100644 --- a/porch/pkg/registry/porch/packagerevision.go +++ b/porch/pkg/registry/porch/packagerevision.go @@ -123,7 +123,7 @@ func (r *packageRevisions) Create(ctx context.Context, runtimeObject runtime.Obj return nil, apierrors.NewBadRequest(fmt.Sprintf("expected PackageRevision object, got %T", runtimeObject)) } - // TODO: Accpept some form of client-provided name, for example using GenerateName + // TODO: Accept some form of client-provided name, for example using GenerateName // and figure out where we can store it (in Kptfile?). Porch can then append unique // suffix to the names while respecting client-provided value as well. if newApiPkgRev.Name != "" { @@ -145,7 +145,16 @@ func (r *packageRevisions) Create(ctx context.Context, runtimeObject runtime.Obj return nil, apierrors.NewInvalid(api.SchemeGroupVersion.WithKind("PackageRevision").GroupKind(), newApiPkgRev.Name, fieldErrors) } - createdRepoPkgRev, err := r.cad.CreatePackageRevision(ctx, repositoryObj, newApiPkgRev) + var parentPackage *engine.PackageRevision + if newApiPkgRev.Spec.Parent != nil && newApiPkgRev.Spec.Parent.Name != "" { + p, err := r.packageCommon.getRepoPkgRev(ctx, newApiPkgRev.Spec.Parent.Name) + if err != nil { + return nil, fmt.Errorf("cannot get parent package %q: %w", newApiPkgRev.Spec.Parent.Name, err) + } + parentPackage = p + } + + createdRepoPkgRev, err := r.cad.CreatePackageRevision(ctx, repositoryObj, newApiPkgRev, parentPackage) if err != nil { return nil, apierrors.NewInternalError(err) }