From 212b6688bd0b1089a0b2e65841c4c71f133ef7c5 Mon Sep 17 00:00:00 2001 From: Morten Torkildsen Date: Fri, 14 Oct 2022 08:35:45 -0700 Subject: [PATCH] Store metadata for packagerevisions in CRs (#3579) --- porch/Makefile | 15 + .../clientset/versioned/fake/register.go | 14 +- .../clientset/versioned/scheme/register.go | 14 +- porch/build/Dockerfile.porch | 1 + porch/deployments/porch/5-rbac.yaml | 3 + .../config.porch.kpt.dev_packagerevs.yaml | 65 +++++ .../v1alpha1/groupversion_info.go | 61 ++++ .../api/porchinternal/v1alpha1/types.go | 49 ++++ .../v1alpha1/zz_generated.deepcopy.go | 113 ++++++++ porch/pkg/apiserver/apiserver.go | 9 + porch/pkg/cache/cache.go | 8 +- porch/pkg/cache/cache_test.go | 46 ++- porch/pkg/cache/draft.go | 2 +- porch/pkg/cache/repository.go | 85 +++++- porch/pkg/engine/clone.go | 4 +- porch/pkg/engine/edit.go | 4 +- porch/pkg/engine/edit_test.go | 56 +--- porch/pkg/engine/engine.go | 270 ++++++++++++++---- porch/pkg/engine/fake/packagerevision.go | 5 + porch/pkg/engine/options.go | 8 + porch/pkg/engine/package.go | 9 +- porch/pkg/git/draft.go | 5 +- porch/pkg/git/package.go | 4 + porch/pkg/meta/fake/memorystore.go | 101 +++++++ porch/pkg/meta/store.go | 229 +++++++++++++++ porch/pkg/oci/oci.go | 4 + porch/pkg/registry/porch/package.go | 4 +- porch/pkg/registry/porch/packagecommon.go | 37 +-- porch/pkg/registry/porch/packagerevision.go | 39 +-- .../registry/porch/packagerevision_test.go | 149 +++++++++- .../porch/packagerevisionresources.go | 26 +- .../porch/packagerevisions_approval.go | 2 +- porch/pkg/registry/porch/strategy.go | 41 ++- porch/pkg/registry/porch/watch.go | 12 +- porch/pkg/repository/repository.go | 2 + porch/scripts/create-deployment-blueprint.sh | 2 + porch/test/e2e/e2e_test.go | 255 +++++++++++++++++ porch/test/e2e/suite.go | 2 + 38 files changed, 1548 insertions(+), 207 deletions(-) create mode 100644 porch/internal/api/porchinternal/v1alpha1/config.porch.kpt.dev_packagerevs.yaml create mode 100644 porch/internal/api/porchinternal/v1alpha1/groupversion_info.go create mode 100644 porch/internal/api/porchinternal/v1alpha1/types.go create mode 100644 porch/internal/api/porchinternal/v1alpha1/zz_generated.deepcopy.go create mode 100644 porch/pkg/meta/fake/memorystore.go create mode 100644 porch/pkg/meta/store.go diff --git a/porch/Makefile b/porch/Makefile index 4c20d44935..1099f35964 100644 --- a/porch/Makefile +++ b/porch/Makefile @@ -152,6 +152,7 @@ PORCH = $(BUILDDIR)/porch run-local: porch KUBECONFIG=$(KUBECONFIG) kubectl apply -f deployments/local/localconfig.yaml KUBECONFIG=$(KUBECONFIG) kubectl apply -f api/porchconfig/v1alpha1/ + KUBECONFIG=$(KUBECONFIG) kubectl apply -f internal/api/porchinternal/v1alpha1/ $(PORCH) \ --secure-port 9443 \ --standalone-debug-mode \ @@ -253,3 +254,17 @@ deploy-no-sa: deployment-config-no-sa .PHONY: push-and-deploy-no-sa push-and-deploy-no-sa: push-images deploy-no-sa + +.PHONY: run-in-kind +run-in-kind: + IMAGE_REPO=porch-kind make build-images + kind load docker-image porch-kind/porch-server:${IMAGE_TAG} + kind load docker-image porch-kind/porch-controllers:${IMAGE_TAG} + kind load docker-image porch-kind/porch-function-runner:${IMAGE_TAG} + kind load docker-image porch-kind/porch-wrapper-server:${IMAGE_TAG} + kind load docker-image porch-kind/test-git-server:${IMAGE_TAG} + IMAGE_REPO=porch-kind make deployment-config + kubectl apply --wait --recursive --filename ./.build/deploy + kubectl rollout status deployment function-runner --namespace porch-system + kubectl rollout status deployment porch-controllers --namespace porch-system + kubectl rollout status deployment porch-server --namespace porch-system diff --git a/porch/api/generated/clientset/versioned/fake/register.go b/porch/api/generated/clientset/versioned/fake/register.go index 2b1d2df776..36b9da3ca7 100644 --- a/porch/api/generated/clientset/versioned/fake/register.go +++ b/porch/api/generated/clientset/versioned/fake/register.go @@ -35,14 +35,14 @@ var localSchemeBuilder = runtime.SchemeBuilder{ // AddToScheme adds all types of this clientset into the given scheme. This allows composition // of clientsets, like in: // -// import ( -// "k8s.io/client-go/kubernetes" -// clientsetscheme "k8s.io/client-go/kubernetes/scheme" -// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme" -// ) +// import ( +// "k8s.io/client-go/kubernetes" +// clientsetscheme "k8s.io/client-go/kubernetes/scheme" +// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme" +// ) // -// kclientset, _ := kubernetes.NewForConfig(c) -// _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme) +// kclientset, _ := kubernetes.NewForConfig(c) +// _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme) // // After this, RawExtensions in Kubernetes types will serialize kube-aggregator types // correctly. diff --git a/porch/api/generated/clientset/versioned/scheme/register.go b/porch/api/generated/clientset/versioned/scheme/register.go index 90fc1da236..cee00d25bb 100644 --- a/porch/api/generated/clientset/versioned/scheme/register.go +++ b/porch/api/generated/clientset/versioned/scheme/register.go @@ -35,14 +35,14 @@ var localSchemeBuilder = runtime.SchemeBuilder{ // AddToScheme adds all types of this clientset into the given scheme. This allows composition // of clientsets, like in: // -// import ( -// "k8s.io/client-go/kubernetes" -// clientsetscheme "k8s.io/client-go/kubernetes/scheme" -// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme" -// ) +// import ( +// "k8s.io/client-go/kubernetes" +// clientsetscheme "k8s.io/client-go/kubernetes/scheme" +// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme" +// ) // -// kclientset, _ := kubernetes.NewForConfig(c) -// _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme) +// kclientset, _ := kubernetes.NewForConfig(c) +// _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme) // // After this, RawExtensions in Kubernetes types will serialize kube-aggregator types // correctly. diff --git a/porch/build/Dockerfile.porch b/porch/build/Dockerfile.porch index 44ff1f156c..5d01c27868 100644 --- a/porch/build/Dockerfile.porch +++ b/porch/build/Dockerfile.porch @@ -48,6 +48,7 @@ COPY pkg pkg COPY porch/api porch/api COPY porch/cmd porch/cmd COPY porch/pkg porch/pkg +COPY porch/internal porch/internal COPY porch/controllers porch/controllers COPY porch/func porch/func diff --git a/porch/deployments/porch/5-rbac.yaml b/porch/deployments/porch/5-rbac.yaml index c08f616074..49563b57e0 100644 --- a/porch/deployments/porch/5-rbac.yaml +++ b/porch/deployments/porch/5-rbac.yaml @@ -27,6 +27,9 @@ rules: - apiGroups: ["config.porch.kpt.dev"] resources: ["repositories", "repositories/status"] verbs: ["get", "list", "watch", "create", "update", "patch"] + - apiGroups: ["config.porch.kpt.dev"] + resources: ["packagerevs", "packagerevs/status"] + verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] # Needed for priority and fairness - apiGroups: ["flowcontrol.apiserver.k8s.io"] resources: ["flowschemas", "prioritylevelconfigurations"] diff --git a/porch/internal/api/porchinternal/v1alpha1/config.porch.kpt.dev_packagerevs.yaml b/porch/internal/api/porchinternal/v1alpha1/config.porch.kpt.dev_packagerevs.yaml new file mode 100644 index 0000000000..1b4d80f919 --- /dev/null +++ b/porch/internal/api/porchinternal/v1alpha1/config.porch.kpt.dev_packagerevs.yaml @@ -0,0 +1,65 @@ +# Copyright 2022 Google LLC +# +# 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. + +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.8.0 + creationTimestamp: null + name: packagerevs.config.porch.kpt.dev +spec: + group: config.porch.kpt.dev + names: + kind: PackageRev + listKind: PackageRevList + plural: packagerevs + singular: packagerev + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: PackageRev + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: PackageRevSpec defines the desired state of PackageRev + type: object + status: + description: PackageRevStatus defines the observed state of PackageRev + type: object + type: object + served: true + storage: true + subresources: + status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/porch/internal/api/porchinternal/v1alpha1/groupversion_info.go b/porch/internal/api/porchinternal/v1alpha1/groupversion_info.go new file mode 100644 index 0000000000..ecde8a652e --- /dev/null +++ b/porch/internal/api/porchinternal/v1alpha1/groupversion_info.go @@ -0,0 +1,61 @@ +// Copyright 2022 Google LLC +// +// 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 v1alpha1 contains API Schema definitions for the v1alpha1 API group +// +kubebuilder:object:generate=true +// +groupName=config.porch.kpt.dev +package v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +//go:generate go run sigs.k8s.io/controller-tools/cmd/controller-gen@v0.8.0 object object:headerFile="../../../../scripts/boilerplate.go.txt" crd:crdVersions=v1 output:crd:artifacts:config=. paths=./... + +var ( + // GroupVersion is group version used to register these objects + GroupVersion = schema.GroupVersion{Group: "config.porch.kpt.dev", Version: "v1alpha1"} + + // We removed SchemeBuilder to keep our dependencies small + + KindRepository = KindInfo{ + Resource: GroupVersion.WithResource("packagerev"), + objects: []runtime.Object{&PackageRev{}, &PackageRevList{}}, + } + + AllKinds = []KindInfo{KindRepository} +) + +//+kubebuilder:object:generate=false + +// KindInfo holds type meta-information +type KindInfo struct { + Resource schema.GroupVersionResource + objects []runtime.Object +} + +// GroupResource returns the GroupResource for the kind +func (k *KindInfo) GroupResource() schema.GroupResource { + return k.Resource.GroupResource() +} + +func AddToScheme(scheme *runtime.Scheme) error { + for _, kind := range AllKinds { + scheme.AddKnownTypes(GroupVersion, kind.objects...) + } + metav1.AddToGroupVersion(scheme, GroupVersion) + return nil +} diff --git a/porch/internal/api/porchinternal/v1alpha1/types.go b/porch/internal/api/porchinternal/v1alpha1/types.go new file mode 100644 index 0000000000..5b11ee8ece --- /dev/null +++ b/porch/internal/api/porchinternal/v1alpha1/types.go @@ -0,0 +1,49 @@ +// Copyright 2022 Google LLC +// +// 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 v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +//+kubebuilder:object:root=true +//+kubebuilder:subresource:status +//+kubebuilder:resource:path=packagerevs,singular=packagerev + +// PackageRev +type PackageRev struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec PackageRevSpec `json:"spec,omitempty"` + Status PackageRevStatus `json:"status,omitempty"` +} + +// PackageRevSpec defines the desired state of PackageRev +type PackageRevSpec struct { +} + +// PackageRevStatus defines the observed state of PackageRev +type PackageRevStatus struct { +} + +//+kubebuilder:object:root=true + +// PackageRevList contains a list of PackageRev +type PackageRevList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []PackageRev `json:"items"` +} diff --git a/porch/internal/api/porchinternal/v1alpha1/zz_generated.deepcopy.go b/porch/internal/api/porchinternal/v1alpha1/zz_generated.deepcopy.go new file mode 100644 index 0000000000..59a90a8780 --- /dev/null +++ b/porch/internal/api/porchinternal/v1alpha1/zz_generated.deepcopy.go @@ -0,0 +1,113 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +// Copyright 2022 Google LLC +// +// 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. + +// Code generated by controller-gen. DO NOT EDIT. + +package v1alpha1 + +import ( + "k8s.io/apimachinery/pkg/runtime" +) + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PackageRev) DeepCopyInto(out *PackageRev) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + out.Spec = in.Spec + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PackageRev. +func (in *PackageRev) DeepCopy() *PackageRev { + if in == nil { + return nil + } + out := new(PackageRev) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *PackageRev) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PackageRevList) DeepCopyInto(out *PackageRevList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]PackageRev, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PackageRevList. +func (in *PackageRevList) DeepCopy() *PackageRevList { + if in == nil { + return nil + } + out := new(PackageRevList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *PackageRevList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PackageRevSpec) DeepCopyInto(out *PackageRevSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PackageRevSpec. +func (in *PackageRevSpec) DeepCopy() *PackageRevSpec { + if in == nil { + return nil + } + out := new(PackageRevSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PackageRevStatus) DeepCopyInto(out *PackageRevStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PackageRevStatus. +func (in *PackageRevStatus) DeepCopy() *PackageRevStatus { + if in == nil { + return nil + } + out := new(PackageRevStatus) + in.DeepCopyInto(out) + return out +} diff --git a/porch/pkg/apiserver/apiserver.go b/porch/pkg/apiserver/apiserver.go index 379c535931..0511520b12 100644 --- a/porch/pkg/apiserver/apiserver.go +++ b/porch/pkg/apiserver/apiserver.go @@ -21,9 +21,11 @@ import ( "github.com/GoogleContainerTools/kpt/internal/fnruntime" "github.com/GoogleContainerTools/kpt/porch/api/porch/install" configapi "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1" + internalapi "github.com/GoogleContainerTools/kpt/porch/internal/api/porchinternal/v1alpha1" "github.com/GoogleContainerTools/kpt/porch/pkg/cache" "github.com/GoogleContainerTools/kpt/porch/pkg/engine" "github.com/GoogleContainerTools/kpt/porch/pkg/kpt" + "github.com/GoogleContainerTools/kpt/porch/pkg/meta" "github.com/GoogleContainerTools/kpt/porch/pkg/registry/porch" "google.golang.org/api/option" "google.golang.org/api/sts/v1" @@ -148,6 +150,9 @@ func (c completedConfig) getCoreClient() (client.WithWatch, error) { if err := corev1.AddToScheme(scheme); err != nil { return nil, fmt.Errorf("error building scheme: %w", err) } + if err := internalapi.AddToScheme(scheme); err != nil { + return nil, fmt.Errorf("error building scheme: %w", err) + } coreClient, err := client.NewWithWatch(restConfig, client.Options{ Scheme: scheme, @@ -199,6 +204,8 @@ func (c completedConfig) New() (*PorchServer, error) { porch.NewGcloudWIResolver(coreV1Client, stsClient), } + metadataStore := meta.NewCrdMetadataStore(coreClient) + credentialResolver := porch.NewCredentialResolver(coreClient, resolverChain) referenceResolver := porch.NewReferenceResolver(coreClient) userInfoProvider := &porch.ApiserverUserInfoProvider{} @@ -213,6 +220,7 @@ func (c completedConfig) New() (*PorchServer, error) { cache := cache.NewCache(c.ExtraConfig.CacheDirectory, cache.CacheOptions{ CredentialResolver: credentialResolver, UserInfoProvider: userInfoProvider, + MetadataStore: metadataStore, }) cad, err := engine.NewCaDEngine( engine.WithCache(cache), @@ -225,6 +233,7 @@ func (c completedConfig) New() (*PorchServer, error) { engine.WithRenderer(renderer), engine.WithReferenceResolver(referenceResolver), engine.WithUserInfoProvider(userInfoProvider), + engine.WithMetadataStore(metadataStore), ) if err != nil { return nil, err diff --git a/porch/pkg/cache/cache.go b/porch/pkg/cache/cache.go index 44b42e39e3..0ed4db6ca5 100644 --- a/porch/pkg/cache/cache.go +++ b/porch/pkg/cache/cache.go @@ -23,6 +23,7 @@ import ( configapi "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1" "github.com/GoogleContainerTools/kpt/porch/pkg/git" + "github.com/GoogleContainerTools/kpt/porch/pkg/meta" "github.com/GoogleContainerTools/kpt/porch/pkg/oci" "github.com/GoogleContainerTools/kpt/porch/pkg/repository" "go.opentelemetry.io/otel/trace" @@ -44,6 +45,7 @@ type Cache struct { cacheDir string credentialResolver repository.CredentialResolver userInfoProvider repository.UserInfoProvider + metadataStore meta.MetadataStore objectCache *objectCache } @@ -51,6 +53,7 @@ type Cache struct { type CacheOptions struct { CredentialResolver repository.CredentialResolver UserInfoProvider repository.UserInfoProvider + MetadataStore meta.MetadataStore } func NewCache(cacheDir string, opts CacheOptions) *Cache { @@ -61,6 +64,7 @@ func NewCache(cacheDir string, opts CacheOptions) *Cache { cacheDir: cacheDir, credentialResolver: opts.CredentialResolver, userInfoProvider: opts.UserInfoProvider, + metadataStore: opts.MetadataStore, objectCache: objectCache, } } @@ -91,7 +95,7 @@ func (c *Cache) OpenRepository(ctx context.Context, repositorySpec *configapi.Re if err != nil { return nil, err } - cr = newRepository(key, r, c.objectCache) + cr = newRepository(key, repositorySpec, r, c.objectCache, c.metadataStore) c.repositories[key] = cr } return cr, nil @@ -127,7 +131,7 @@ func (c *Cache) OpenRepository(ctx context.Context, repositorySpec *configapi.Re }); err != nil { return nil, err } else { - cr = newRepository(key, r, c.objectCache) + cr = newRepository(key, repositorySpec, r, c.objectCache, c.metadataStore) c.repositories[key] = cr } } else { diff --git a/porch/pkg/cache/cache_test.go b/porch/pkg/cache/cache_test.go index d1b434c7ec..fbe4187369 100644 --- a/porch/pkg/cache/cache_test.go +++ b/porch/pkg/cache/cache_test.go @@ -16,22 +16,28 @@ package cache import ( "context" + "fmt" + "os" "path/filepath" "testing" api "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1" "github.com/GoogleContainerTools/kpt/porch/pkg/git" + "github.com/GoogleContainerTools/kpt/porch/pkg/meta" + "github.com/GoogleContainerTools/kpt/porch/pkg/meta/fake" "github.com/GoogleContainerTools/kpt/porch/pkg/repository" gogit "github.com/go-git/go-git/v5" "github.com/google/go-cmp/cmp" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/yaml" ) func TestLatestPackages(t *testing.T) { ctx := context.Background() - tarfile := filepath.Join("..", "git", "testdata", "nested-repository.tar") - _, cachedGit := openRepositoryFromArchive(t, ctx, tarfile, "nested") + testPath := filepath.Join("..", "git", "testdata") + + _, cachedGit := openRepositoryFromArchive(t, ctx, testPath, "nested") wantLatest := map[string]string{ "sample": "v2", @@ -72,8 +78,8 @@ func TestLatestPackages(t *testing.T) { func TestPublishedLatest(t *testing.T) { ctx := context.Background() - tarfile := filepath.Join("..", "git", "testdata", "nested-repository.tar") - _, cached := openRepositoryFromArchive(t, ctx, tarfile, "publish-test") + testPath := filepath.Join("..", "git", "testdata") + _, cached := openRepositoryFromArchive(t, ctx, testPath, "nested") revisions, err := cached.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{ Package: "catalog/gcp/bucket", @@ -113,13 +119,17 @@ func TestPublishedLatest(t *testing.T) { } } -func openRepositoryFromArchive(t *testing.T, ctx context.Context, tarfile, name string) (*gogit.Repository, *cachedRepository) { +func openRepositoryFromArchive(t *testing.T, ctx context.Context, testPath, name string) (*gogit.Repository, *cachedRepository) { t.Helper() tempdir := t.TempDir() + tarfile := filepath.Join(testPath, fmt.Sprintf("%s-repository.tar", name)) repo, address := git.ServeGitRepository(t, tarfile, tempdir) + metadataStore := createMetadataStoreFromArchive(t, "", "") - cache := NewCache(t.TempDir(), CacheOptions{}) + cache := NewCache(t.TempDir(), CacheOptions{ + MetadataStore: metadataStore, + }) cachedGit, err := cache.OpenRepository(ctx, &v1alpha1.Repository{ TypeMeta: metav1.TypeMeta{ Kind: v1alpha1.RepositoryGVK.Kind, @@ -144,3 +154,27 @@ func openRepositoryFromArchive(t *testing.T, ctx context.Context, tarfile, name return repo, cachedGit } + +func createMetadataStoreFromArchive(t *testing.T, testPath, name string) meta.MetadataStore { + t.Helper() + + f := filepath.Join("..", "git", "testdata", "nested-metadata.yaml") + c, err := os.ReadFile(f) + if err != nil && !os.IsNotExist(err) { + t.Fatalf("Error reading metadata file found for repository %s", name) + } + if os.IsNotExist(err) { + return &fake.MemoryMetadataStore{ + Metas: []meta.PackageRevisionMeta{}, + } + } + + var metas []meta.PackageRevisionMeta + if err := yaml.Unmarshal(c, &metas); err != nil { + t.Fatalf("Error unmarshalling metadata file for repository %s", name) + } + + return &fake.MemoryMetadataStore{ + Metas: metas, + } +} diff --git a/porch/pkg/cache/draft.go b/porch/pkg/cache/draft.go index c6bef5ef3d..afc8855c21 100644 --- a/porch/pkg/cache/draft.go +++ b/porch/pkg/cache/draft.go @@ -31,6 +31,6 @@ func (cd *cachedDraft) Close(ctx context.Context) (repository.PackageRevision, e if closed, err := cd.PackageDraft.Close(ctx); err != nil { return nil, err } else { - return cd.cache.update(closed) + return cd.cache.update(ctx, closed) } } diff --git a/porch/pkg/cache/repository.go b/porch/pkg/cache/repository.go index 1837204bf2..a019a21084 100644 --- a/porch/pkg/cache/repository.go +++ b/porch/pkg/cache/repository.go @@ -21,9 +21,13 @@ import ( "time" "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" + configapi "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1" + "github.com/GoogleContainerTools/kpt/porch/pkg/meta" "github.com/GoogleContainerTools/kpt/porch/pkg/repository" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/trace" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" "k8s.io/klog/v2" ) @@ -40,9 +44,12 @@ var _ repository.Repository = &cachedRepository{} var _ repository.FunctionRepository = &cachedRepository{} type cachedRepository struct { - id string - repo repository.Repository - cancel context.CancelFunc + id string + // We need the kubernetes object so we can add the appropritate + // ownerreferences to PackageRevision resources. + repoSpec *configapi.Repository + repo repository.Repository + cancel context.CancelFunc mutex sync.Mutex cachedPackageRevisions map[repository.PackageRevisionKey]*cachedPackageRevision @@ -56,15 +63,19 @@ type cachedRepository struct { refreshPkgsError error objectCache *objectCache + + metadataStore meta.MetadataStore } -func newRepository(id string, repo repository.Repository, objectCache *objectCache) *cachedRepository { +func newRepository(id string, repoSpec *configapi.Repository, repo repository.Repository, objectCache *objectCache, metadataStore meta.MetadataStore) *cachedRepository { ctx, cancel := context.WithCancel(context.Background()) r := &cachedRepository{ - id: id, - repo: repo, - cancel: cancel, - objectCache: objectCache, + id: id, + repoSpec: repoSpec, + repo: repo, + cancel: cancel, + objectCache: objectCache, + metadataStore: metadataStore, } // TODO: Should we fetch the packages here? @@ -200,12 +211,12 @@ func (r *cachedRepository) UpdatePackageRevision(ctx context.Context, old reposi }, nil } -func (r *cachedRepository) update(updated repository.PackageRevision) (*cachedPackageRevision, error) { +func (r *cachedRepository) update(ctx context.Context, updated repository.PackageRevision) (*cachedPackageRevision, error) { r.mutex.Lock() defer r.mutex.Unlock() // TODO: Technically we only need this package, not all packages - if _, _, err := r.getCachedPackages(context.TODO(), false); err != nil { + if _, _, err := r.getCachedPackages(ctx, false); err != nil { klog.Warningf("failed to get cached packages: %v", err) // TODO: Invalidate all watches? We're dropping an add/update event return nil, err @@ -283,7 +294,6 @@ func (r *cachedRepository) Close() error { // pollForever will continue polling until signal channel is closed or ctx is done. func (r *cachedRepository) pollForever(ctx context.Context) { ticker := time.NewTicker(1 * time.Minute) - for { select { case <-ticker.C: @@ -328,6 +338,19 @@ func (r *cachedRepository) refreshAllCachedPackages(ctx context.Context) (map[re // TODO: Avoid simultaneous fetches? // TODO: Push-down partial refresh? + // Look up all existing PackageRevCRs so we an compare those to the + // actual Packagerevisions found in git/oci, and add/prune PackageRevCRs + // as necessary. + existingPkgRevCRs, err := r.metadataStore.List(ctx, r.repoSpec) + if err != nil { + return nil, nil, err + } + // Create a map so we can quickly check if a specific PackageRevisionMeta exists. + existingPkgRevCRsMap := make(map[string]bool) + for _, pr := range existingPkgRevCRs { + existingPkgRevCRsMap[pr.Name] = true + } + // TODO: Can we avoid holding the lock for the ListPackageRevisions / identifyLatestRevisions section? newPackageRevisions, err := r.repo.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{}) if err != nil { @@ -335,15 +358,18 @@ func (r *cachedRepository) refreshAllCachedPackages(ctx context.Context) (map[re } newPackageRevisionMap := make(map[repository.PackageRevisionKey]*cachedPackageRevision, len(newPackageRevisions)) + newPackageRevisionNames := make(map[string]bool) for _, newPackage := range newPackageRevisions { k := newPackage.Key() if newPackageRevisionMap[k] != nil { klog.Warningf("found duplicate packages with key %v", k) } + newPackageRevisionMap[k] = &cachedPackageRevision{ PackageRevision: newPackage, isLatestRevision: false, } + newPackageRevisionNames[newPackage.KubeObjectName()] = true } identifyLatestRevisions(newPackageRevisionMap) @@ -364,6 +390,43 @@ func (r *cachedRepository) refreshAllCachedPackages(ctx context.Context) (map[re r.cachedPackageRevisions = newPackageRevisionMap r.cachedPackages = newPackageMap + // We go through all PackageRev CRs that represents PackageRevisions + // in the current repo and make sure they all have a corresponding + // PackageRevision. The ones that doesn't is removed. + for _, prm := range existingPkgRevCRs { + if _, found := newPackageRevisionNames[prm.Name]; !found { + if _, err := r.metadataStore.Delete(ctx, types.NamespacedName{ + Name: prm.Name, + Namespace: prm.Namespace, + }); err != nil { + if !apierrors.IsNotFound(err) { + // This will be retried the next time the sync runs. + klog.Warningf("unable to create PackageRev CR for %s/%s: %w", + prm.Name, prm.Namespace, err) + } + } + } + } + + // We go through all the PackageRevisions and make sure they have + // a corresponding PackageRev CR. + for pkgRevName := range newPackageRevisionNames { + if _, found := existingPkgRevCRsMap[pkgRevName]; !found { + pkgRevMeta := meta.PackageRevisionMeta{ + Name: pkgRevName, + Namespace: r.repoSpec.Namespace, + } + if _, err := r.metadataStore.Create(ctx, pkgRevMeta, r.repoSpec); err != nil { + // TODO: We should try to find a way to make these errors available through + // either the repository CR or the PackageRevision CR. This will be + // retried on the next sync. + klog.Warningf("unable to create PackageRev CR for %s/%s: %w", + r.repoSpec.Namespace, pkgRevName, err) + } + } + } + + // Send notification for packages that changed. for k, newPackage := range r.cachedPackageRevisions { oldPackage := oldPackageRevisions[k] if oldPackage == nil { diff --git a/porch/pkg/engine/clone.go b/porch/pkg/engine/clone.go index 4d5af71e7a..fa53f2dd03 100644 --- a/porch/pkg/engine/clone.go +++ b/porch/pkg/engine/clone.go @@ -41,7 +41,7 @@ type clonePackageMutation struct { name string // package target name isDeployment bool // is the package deployable instance - cad CaDEngine + repoOpener RepositoryOpener credentialResolver repository.CredentialResolver referenceResolver ReferenceResolver } @@ -105,7 +105,7 @@ func (m *clonePackageMutation) cloneFromRegisteredRepository(ctx context.Context } upstreamRevision, err := (&PackageFetcher{ - cad: m.cad, + repoOpener: m.repoOpener, referenceResolver: m.referenceResolver, }).FetchRevision(ctx, ref, m.namespace) if err != nil { diff --git a/porch/pkg/engine/edit.go b/porch/pkg/engine/edit.go index 6d96e1d6d3..abf2c8a9f5 100644 --- a/porch/pkg/engine/edit.go +++ b/porch/pkg/engine/edit.go @@ -26,7 +26,7 @@ import ( type editPackageMutation struct { task *api.Task namespace string - cad CaDEngine + repoOpener RepositoryOpener referenceResolver ReferenceResolver } @@ -39,7 +39,7 @@ func (m *editPackageMutation) Apply(ctx context.Context, resources repository.Pa sourceRef := m.task.Edit.Source sourceResources, err := (&PackageFetcher{ - cad: m.cad, + repoOpener: m.repoOpener, referenceResolver: m.referenceResolver, }).FetchResources(ctx, sourceRef, m.namespace) if err != nil { diff --git a/porch/pkg/engine/edit_test.go b/porch/pkg/engine/edit_test.go index 279c558b2f..b02812dba1 100644 --- a/porch/pkg/engine/edit_test.go +++ b/porch/pkg/engine/edit_test.go @@ -22,7 +22,6 @@ import ( kptfile "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1" "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" configapi "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1" - "github.com/GoogleContainerTools/kpt/porch/pkg/cache" "github.com/GoogleContainerTools/kpt/porch/pkg/engine/fake" "github.com/GoogleContainerTools/kpt/porch/pkg/repository" "github.com/google/go-cmp/cmp" @@ -57,8 +56,7 @@ info: packageRevision, }, } - cad := &fakeCaD{ - cache: cache.NewCache("", cache.CacheOptions{}), + repoOpener := &fakeRepositoryOpener{ repository: repo, } @@ -73,7 +71,7 @@ info: }, namespace: "test-namespace", referenceResolver: &fakeReferenceResolver{}, - cad: cad, + repoOpener: repoOpener, } res, _, err := epm.Apply(context.Background(), repository.PackageResources{}) @@ -104,54 +102,10 @@ func (f *fakeReferenceResolver) ResolveReference(ctx context.Context, namespace, return nil } -// Implementation of the engine.CaDEngine interface for testing. -type fakeCaD struct { - cache *cache.Cache +type fakeRepositoryOpener struct { repository repository.Repository } -var _ CaDEngine = &fakeCaD{} - -func (f *fakeCaD) ObjectCache() cache.ObjectCache { - return f.cache.ObjectCache() -} - -func (f *fakeCaD) ListPackageRevisions(ctx context.Context, _ *configapi.Repository, filter repository.ListPackageRevisionFilter) ([]repository.PackageRevision, error) { - return f.repository.ListPackageRevisions(ctx, filter) -} - -func (f *fakeCaD) CreatePackageRevision(context.Context, *configapi.Repository, *v1alpha1.PackageRevision) (repository.PackageRevision, error) { - return nil, nil -} - -func (f *fakeCaD) UpdatePackageRevision(_ context.Context, _ *configapi.Repository, _ repository.PackageRevision, _, _ *v1alpha1.PackageRevision) (repository.PackageRevision, error) { - return nil, nil -} - -func (f *fakeCaD) UpdatePackageResources(_ context.Context, _ *configapi.Repository, _ repository.PackageRevision, _, _ *v1alpha1.PackageRevisionResources) (repository.PackageRevision, error) { - return nil, nil -} - -func (f *fakeCaD) DeletePackageRevision(context.Context, *configapi.Repository, repository.PackageRevision) error { - return nil -} - -func (f *fakeCaD) ListFunctions(context.Context, *configapi.Repository) ([]repository.Function, error) { - return []repository.Function{}, nil -} - -func (f *fakeCaD) ListPackages(ctx context.Context, _ *configapi.Repository, filter repository.ListPackageFilter) ([]repository.Package, error) { - return f.repository.ListPackages(ctx, filter) -} - -func (f *fakeCaD) CreatePackage(context.Context, *configapi.Repository, *v1alpha1.Package) (repository.Package, error) { - return nil, nil -} - -func (f *fakeCaD) UpdatePackage(_ context.Context, _ *configapi.Repository, _ repository.Package, _, _ *v1alpha1.Package) (repository.Package, error) { - return nil, nil -} - -func (f *fakeCaD) DeletePackage(context.Context, *configapi.Repository, repository.Package) error { - return nil +func (f *fakeRepositoryOpener) OpenRepository(ctx context.Context, repositorySpec *configapi.Repository) (repository.Repository, error) { + return f.repository, nil } diff --git a/porch/pkg/engine/engine.go b/porch/pkg/engine/engine.go index 4669bbd1d5..f6faf9226b 100644 --- a/porch/pkg/engine/engine.go +++ b/porch/pkg/engine/engine.go @@ -27,9 +27,12 @@ import ( configapi "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1" "github.com/GoogleContainerTools/kpt/porch/pkg/cache" "github.com/GoogleContainerTools/kpt/porch/pkg/kpt" + "github.com/GoogleContainerTools/kpt/porch/pkg/meta" "github.com/GoogleContainerTools/kpt/porch/pkg/repository" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/trace" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2" "sigs.k8s.io/kustomize/kyaml/comments" "sigs.k8s.io/kustomize/kyaml/kio" @@ -42,18 +45,72 @@ type CaDEngine interface { // ObjectCache() is a cache of all our objects. ObjectCache() cache.ObjectCache - UpdatePackageResources(ctx context.Context, repositoryObj *configapi.Repository, oldPackage repository.PackageRevision, old, new *api.PackageRevisionResources) (repository.PackageRevision, error) - ListFunctions(ctx context.Context, repositoryObj *configapi.Repository) ([]repository.Function, error) + UpdatePackageResources(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, old, new *api.PackageRevisionResources) (*PackageRevision, error) + ListFunctions(ctx context.Context, repositoryObj *configapi.Repository) ([]*Function, error) - ListPackageRevisions(ctx context.Context, repositorySpec *configapi.Repository, filter repository.ListPackageRevisionFilter) ([]repository.PackageRevision, error) - CreatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, obj *api.PackageRevision) (repository.PackageRevision, error) - UpdatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, oldPackage repository.PackageRevision, old, new *api.PackageRevision) (repository.PackageRevision, error) - DeletePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, obj repository.PackageRevision) 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) + DeletePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, obj *PackageRevision) error - ListPackages(ctx context.Context, repositorySpec *configapi.Repository, filter repository.ListPackageFilter) ([]repository.Package, error) - CreatePackage(ctx context.Context, repositoryObj *configapi.Repository, obj *api.Package) (repository.Package, error) - UpdatePackage(ctx context.Context, repositoryObj *configapi.Repository, oldPackage repository.Package, old, new *api.Package) (repository.Package, error) - DeletePackage(ctx context.Context, repositoryObj *configapi.Repository, obj repository.Package) error + ListPackages(ctx context.Context, repositorySpec *configapi.Repository, filter repository.ListPackageFilter) ([]*Package, error) + CreatePackage(ctx context.Context, repositoryObj *configapi.Repository, obj *api.Package) (*Package, error) + UpdatePackage(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *Package, old, new *api.Package) (*Package, error) + DeletePackage(ctx context.Context, repositoryObj *configapi.Repository, obj *Package) error +} + +type Package struct { + repoPackage repository.Package +} + +func (p *Package) GetPackage() *api.Package { + return p.repoPackage.GetPackage() +} + +func (p *Package) KubeObjectName() string { + return p.repoPackage.KubeObjectName() +} + +type PackageRevision struct { + repoPackageRevision repository.PackageRevision + packageRevisionMeta meta.PackageRevisionMeta +} + +func (p *PackageRevision) GetPackageRevision() *api.PackageRevision { + repoPkgRev := p.repoPackageRevision.GetPackageRevision() + var isLatest bool + if val, found := repoPkgRev.Labels[api.LatestPackageRevisionKey]; found && val == api.LatestPackageRevisionValue { + isLatest = true + } + repoPkgRev.Labels = p.packageRevisionMeta.Labels + if isLatest { + if repoPkgRev.Labels == nil { + repoPkgRev.Labels = make(map[string]string) + } + repoPkgRev.Labels[api.LatestPackageRevisionKey] = api.LatestPackageRevisionValue + } + repoPkgRev.Annotations = p.packageRevisionMeta.Annotations + return repoPkgRev +} + +func (p *PackageRevision) KubeObjectName() string { + return p.repoPackageRevision.KubeObjectName() +} + +func (p *PackageRevision) GetResources(ctx context.Context) (*api.PackageRevisionResources, error) { + return p.repoPackageRevision.GetResources(ctx) +} + +type Function struct { + RepoFunction repository.Function +} + +func (f *Function) Name() string { + return f.RepoFunction.Name() +} + +func (f *Function) GetFunction() (*api.Function, error) { + return f.RepoFunction.GetFunction() } func NewCaDEngine(opts ...EngineOption) (CaDEngine, error) { @@ -73,6 +130,7 @@ type cadEngine struct { credentialResolver repository.CredentialResolver referenceResolver ReferenceResolver userInfoProvider repository.UserInfoProvider + metadataStore meta.MetadataStore } var _ CaDEngine = &cadEngine{} @@ -93,7 +151,7 @@ func (cad *cadEngine) OpenRepository(ctx context.Context, repositorySpec *config return cad.cache.OpenRepository(ctx, repositorySpec) } -func (cad *cadEngine) ListPackageRevisions(ctx context.Context, repositorySpec *configapi.Repository, filter repository.ListPackageRevisionFilter) ([]repository.PackageRevision, error) { +func (cad *cadEngine) ListPackageRevisions(ctx context.Context, repositorySpec *configapi.Repository, filter repository.ListPackageRevisionFilter) ([]*PackageRevision, error) { ctx, span := tracer.Start(ctx, "cadEngine::ListPackageRevisions", trace.WithAttributes()) defer span.End() @@ -101,10 +159,34 @@ func (cad *cadEngine) ListPackageRevisions(ctx context.Context, repositorySpec * if err != nil { return nil, err } - return repo.ListPackageRevisions(ctx, filter) + pkgRevs, err := repo.ListPackageRevisions(ctx, filter) + if err != nil { + return nil, err + } + + var packageRevisions []*PackageRevision + for _, pr := range pkgRevs { + pkgRevMeta, err := cad.metadataStore.Get(ctx, types.NamespacedName{ + Name: pr.KubeObjectName(), + Namespace: pr.KubeObjectNamespace(), + }) + if err != nil { + // If a PackageRev CR doesn't exist, we treat the + // Packagerevision as not existing. + if apierrors.IsNotFound(err) { + continue + } + return nil, err + } + packageRevisions = append(packageRevisions, &PackageRevision{ + repoPackageRevision: pr, + packageRevisionMeta: pkgRevMeta, + }) + } + return packageRevisions, nil } -func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, obj *api.PackageRevision) (repository.PackageRevision, error) { +func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, obj *api.PackageRevision) (*PackageRevision, error) { ctx, span := tracer.Start(ctx, "cadEngine::CreatePackageRevision", trace.WithAttributes()) defer span.End() @@ -140,7 +222,24 @@ func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj * } // Updates are done. - return draft.Close(ctx) + repoPkgRev, err := draft.Close(ctx) + if err != nil { + return nil, err + } + pkgRevMeta := meta.PackageRevisionMeta{ + Name: repoPkgRev.KubeObjectName(), + Namespace: repoPkgRev.KubeObjectNamespace(), + Labels: obj.Labels, + Annotations: obj.Annotations, + } + pkgRevMeta, err = cad.metadataStore.Create(ctx, pkgRevMeta, repositoryObj) + if err != nil { + return nil, err + } + return &PackageRevision{ + repoPackageRevision: repoPkgRev, + packageRevisionMeta: pkgRevMeta, + }, nil } func (cad *cadEngine) applyTasks(ctx context.Context, draft repository.PackageDraft, repositoryObj *configapi.Repository, obj *api.PackageRevision) error { @@ -180,6 +279,10 @@ func (cad *cadEngine) applyTasks(ctx context.Context, draft repository.PackageDr return nil } +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) { switch task.Type { case api.TaskTypeInit: @@ -199,7 +302,7 @@ func (cad *cadEngine) mapTaskToMutation(ctx context.Context, obj *api.PackageRev namespace: obj.Namespace, name: obj.Spec.PackageName, isDeployment: isDeployment, - cad: cad, + repoOpener: cad, credentialResolver: cad.credentialResolver, referenceResolver: cad.referenceResolver, }, nil @@ -216,7 +319,7 @@ func (cad *cadEngine) mapTaskToMutation(ctx context.Context, obj *api.PackageRev cloneTask: cloneTask, updateTask: task, namespace: obj.Namespace, - cad: cad, + repoOpener: cad, referenceResolver: cad.referenceResolver, pkgName: obj.Spec.PackageName, }, nil @@ -231,7 +334,7 @@ func (cad *cadEngine) mapTaskToMutation(ctx context.Context, obj *api.PackageRev return &editPackageMutation{ task: task, namespace: obj.Namespace, - cad: cad, + repoOpener: cad, referenceResolver: cad.referenceResolver, }, nil @@ -258,10 +361,15 @@ func (cad *cadEngine) mapTaskToMutation(ctx context.Context, obj *api.PackageRev } } -func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, oldPackage repository.PackageRevision, oldObj, newObj *api.PackageRevision) (repository.PackageRevision, error) { +func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, oldObj, newObj *api.PackageRevision) (*PackageRevision, error) { ctx, span := tracer.Start(ctx, "cadEngine::UpdatePackageRevision", trace.WithAttributes()) defer span.End() + repo, err := cad.cache.OpenRepository(ctx, repositoryObj) + if err != nil { + return nil, err + } + // Validate package lifecycle. Can only update a draft. switch lifecycle := oldObj.Spec.Lifecycle; lifecycle { default: @@ -269,8 +377,21 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj * case api.PackageRevisionLifecycleDraft, api.PackageRevisionLifecycleProposed: // Draft or proposed can be updated. case api.PackageRevisionLifecyclePublished: - // TODO: generate errors that can be translated to correct HTTP responses - return nil, fmt.Errorf("cannot update a package revision with lifecycle value %q", lifecycle) + // Only metadata (currently labels and annotations) can be updated for published packages. + repoPkgRev := oldPackage.repoPackageRevision + + pkgRevMeta := meta.PackageRevisionMeta{ + Name: repoPkgRev.KubeObjectName(), + Namespace: repoPkgRev.KubeObjectNamespace(), + Labels: newObj.Labels, + Annotations: newObj.Annotations, + } + cad.metadataStore.Update(ctx, pkgRevMeta) + + return &PackageRevision{ + repoPackageRevision: repoPkgRev, + packageRevisionMeta: pkgRevMeta, + }, nil } switch lifecycle := newObj.Spec.Lifecycle; lifecycle { default: @@ -279,13 +400,14 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj * // These values are ok } - repo, err := cad.cache.OpenRepository(ctx, repositoryObj) - if err != nil { - return nil, err - } - if isRecloneAndReplay(oldObj, newObj) { - return cad.recloneAndReplay(ctx, repo, repositoryObj, newObj) + repoPkgRev, err := cad.recloneAndReplay(ctx, repo, repositoryObj, newObj) + if err != nil { + return nil, err + } + return &PackageRevision{ + repoPackageRevision: repoPkgRev, + }, nil } var mutations []mutation @@ -320,7 +442,7 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj * mutation := &updatePackageMutation{ cloneTask: cloneTask, updateTask: &newTask, - cad: cad, + repoOpener: cad, referenceResolver: cad.referenceResolver, namespace: repositoryObj.Namespace, pkgName: oldObj.GetName(), @@ -331,7 +453,7 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj * // Re-render if we are making changes. mutations = cad.conditionalAddRender(mutations) - draft, err := repo.UpdatePackageRevision(ctx, oldPackage) + draft, err := repo.UpdatePackageRevision(ctx, oldPackage.repoPackageRevision) if err != nil { return nil, err } @@ -357,7 +479,23 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj * } // Updates are done. - return draft.Close(ctx) + repoPkgRev, err := draft.Close(ctx) + if err != nil { + return nil, err + } + + pkgRevMeta := meta.PackageRevisionMeta{ + Name: repoPkgRev.KubeObjectName(), + Namespace: repoPkgRev.KubeObjectNamespace(), + Labels: newObj.Labels, + Annotations: newObj.Annotations, + } + cad.metadataStore.Update(ctx, pkgRevMeta) + + return &PackageRevision{ + repoPackageRevision: repoPkgRev, + packageRevisionMeta: pkgRevMeta, + }, nil } // conditionalAddRender adds a render mutation to the end of the mutations slice if the last @@ -379,7 +517,7 @@ func (cad *cadEngine) conditionalAddRender(mutations []mutation) []mutation { }) } -func (cad *cadEngine) DeletePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, oldPackage repository.PackageRevision) error { +func (cad *cadEngine) DeletePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision) error { ctx, span := tracer.Start(ctx, "cadEngine::DeletePackageRevision", trace.WithAttributes()) defer span.End() @@ -388,14 +526,22 @@ func (cad *cadEngine) DeletePackageRevision(ctx context.Context, repositoryObj * return err } - if err := repo.DeletePackageRevision(ctx, oldPackage); err != nil { + if err := repo.DeletePackageRevision(ctx, oldPackage.repoPackageRevision); err != nil { + return err + } + + namespacedName := types.NamespacedName{ + Name: oldPackage.repoPackageRevision.KubeObjectName(), + Namespace: oldPackage.repoPackageRevision.KubeObjectNamespace(), + } + if _, err := cad.metadataStore.Delete(ctx, namespacedName); err != nil { return err } return nil } -func (cad *cadEngine) ListPackages(ctx context.Context, repositorySpec *configapi.Repository, filter repository.ListPackageFilter) ([]repository.Package, error) { +func (cad *cadEngine) ListPackages(ctx context.Context, repositorySpec *configapi.Repository, filter repository.ListPackageFilter) ([]*Package, error) { ctx, span := tracer.Start(ctx, "cadEngine::ListPackages", trace.WithAttributes()) defer span.End() @@ -404,10 +550,21 @@ func (cad *cadEngine) ListPackages(ctx context.Context, repositorySpec *configap return nil, err } - return repo.ListPackages(ctx, filter) + pkgs, err := repo.ListPackages(ctx, filter) + if err != nil { + return nil, err + } + var packages []*Package + for _, p := range pkgs { + packages = append(packages, &Package{ + repoPackage: p, + }) + } + + return packages, nil } -func (cad *cadEngine) CreatePackage(ctx context.Context, repositoryObj *configapi.Repository, obj *api.Package) (repository.Package, error) { +func (cad *cadEngine) CreatePackage(ctx context.Context, repositoryObj *configapi.Repository, obj *api.Package) (*Package, error) { ctx, span := tracer.Start(ctx, "cadEngine::CreatePackage", trace.WithAttributes()) defer span.End() @@ -420,19 +577,21 @@ func (cad *cadEngine) CreatePackage(ctx context.Context, repositoryObj *configap return nil, err } - return pkg, nil + return &Package{ + repoPackage: pkg, + }, nil } -func (cad *cadEngine) UpdatePackage(ctx context.Context, repositoryObj *configapi.Repository, oldPackage repository.Package, oldObj, newObj *api.Package) (repository.Package, error) { +func (cad *cadEngine) UpdatePackage(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *Package, oldObj, newObj *api.Package) (*Package, error) { ctx, span := tracer.Start(ctx, "cadEngine::UpdatePackage", trace.WithAttributes()) defer span.End() // TODO - var pkg repository.Package + var pkg *Package return pkg, fmt.Errorf("Updating packages is not yet supported") } -func (cad *cadEngine) DeletePackage(ctx context.Context, repositoryObj *configapi.Repository, oldPackage repository.Package) error { +func (cad *cadEngine) DeletePackage(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *Package) error { ctx, span := tracer.Start(ctx, "cadEngine::DeletePackage", trace.WithAttributes()) defer span.End() @@ -441,18 +600,18 @@ func (cad *cadEngine) DeletePackage(ctx context.Context, repositoryObj *configap return err } - if err := repo.DeletePackage(ctx, oldPackage); err != nil { + if err := repo.DeletePackage(ctx, oldPackage.repoPackage); err != nil { return err } return nil } -func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj *configapi.Repository, oldPackage repository.PackageRevision, old, new *api.PackageRevisionResources) (repository.PackageRevision, error) { +func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, old, new *api.PackageRevisionResources) (*PackageRevision, error) { ctx, span := tracer.Start(ctx, "cadEngine::UpdatePackageResources", trace.WithAttributes()) defer span.End() - rev := oldPackage.GetPackageRevision() + rev := oldPackage.repoPackageRevision.GetPackageRevision() // Validate package lifecycle. Can only update a draft. switch lifecycle := rev.Spec.Lifecycle; lifecycle { @@ -470,7 +629,7 @@ func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj return nil, err } - draft, err := repo.UpdatePackageRevision(ctx, oldPackage) + draft, err := repo.UpdatePackageRevision(ctx, oldPackage.repoPackageRevision) if err != nil { return nil, err } @@ -486,7 +645,7 @@ func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj }, } - apiResources, err := oldPackage.GetResources(ctx) + apiResources, err := oldPackage.repoPackageRevision.GetResources(ctx) if err != nil { return nil, fmt.Errorf("cannot get package resources: %w", err) } @@ -499,7 +658,13 @@ func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj } // No lifecycle change when updating package resources; updates are done. - return draft.Close(ctx) + repoPkgRev, err := draft.Close(ctx) + if err != nil { + return nil, err + } + return &PackageRevision{ + repoPackageRevision: repoPkgRev, + }, nil } func applyResourceMutations(ctx context.Context, draft repository.PackageDraft, baseResources repository.PackageResources, mutations []mutation) error { @@ -521,7 +686,7 @@ func applyResourceMutations(ctx context.Context, draft repository.PackageDraft, return nil } -func (cad *cadEngine) ListFunctions(ctx context.Context, repositoryObj *configapi.Repository) ([]repository.Function, error) { +func (cad *cadEngine) ListFunctions(ctx context.Context, repositoryObj *configapi.Repository) ([]*Function, error) { ctx, span := tracer.Start(ctx, "cadEngine::ListFunctions", trace.WithAttributes()) defer span.End() @@ -535,13 +700,20 @@ func (cad *cadEngine) ListFunctions(ctx context.Context, repositoryObj *configap return nil, err } - return fns, nil + var functions []*Function + for _, f := range fns { + functions = append(functions, &Function{ + RepoFunction: f, + }) + } + + return functions, nil } type updatePackageMutation struct { cloneTask *api.Task updateTask *api.Task - cad CaDEngine + repoOpener RepositoryOpener referenceResolver ReferenceResolver namespace string pkgName string @@ -562,7 +734,7 @@ func (m *updatePackageMutation) Apply(ctx context.Context, resources repository. } originalResources, err := (&PackageFetcher{ - cad: m.cad, + repoOpener: m.repoOpener, referenceResolver: m.referenceResolver, }).FetchResources(ctx, currUpstreamPkgRef, m.namespace) if err != nil { @@ -571,7 +743,7 @@ func (m *updatePackageMutation) Apply(ctx context.Context, resources repository. } upstreamRevision, err := (&PackageFetcher{ - cad: m.cad, + repoOpener: m.repoOpener, referenceResolver: m.referenceResolver, }).FetchRevision(ctx, targetUpstream.UpstreamRef, m.namespace) if err != nil { diff --git a/porch/pkg/engine/fake/packagerevision.go b/porch/pkg/engine/fake/packagerevision.go index e87088513c..2cfe5fa908 100644 --- a/porch/pkg/engine/fake/packagerevision.go +++ b/porch/pkg/engine/fake/packagerevision.go @@ -25,6 +25,7 @@ import ( // Implementation of the repository.PackageRevision interface for testing. type PackageRevision struct { Name string + Namespace string PackageRevisionKey repository.PackageRevisionKey PackageLifecycle v1alpha1.PackageRevisionLifecycle PackageRevision *v1alpha1.PackageRevision @@ -37,6 +38,10 @@ func (pr *PackageRevision) KubeObjectName() string { return pr.Name } +func (pr *PackageRevision) KubeObjectNamespace() string { + return pr.Namespace +} + func (pr *PackageRevision) Key() repository.PackageRevisionKey { return pr.PackageRevisionKey } diff --git a/porch/pkg/engine/options.go b/porch/pkg/engine/options.go index 582bc18b6b..0960fc60c1 100644 --- a/porch/pkg/engine/options.go +++ b/porch/pkg/engine/options.go @@ -20,6 +20,7 @@ import ( "github.com/GoogleContainerTools/kpt/pkg/fn" "github.com/GoogleContainerTools/kpt/porch/pkg/cache" "github.com/GoogleContainerTools/kpt/porch/pkg/kpt" + "github.com/GoogleContainerTools/kpt/porch/pkg/meta" "github.com/GoogleContainerTools/kpt/porch/pkg/repository" ) @@ -114,3 +115,10 @@ func WithUserInfoProvider(provider repository.UserInfoProvider) EngineOption { return nil }) } + +func WithMetadataStore(metadataStore meta.MetadataStore) EngineOption { + return EngineOptionFunc(func(engine *cadEngine) error { + engine.metadataStore = metadataStore + return nil + }) +} diff --git a/porch/pkg/engine/package.go b/porch/pkg/engine/package.go index eb4ccf32b2..aafcd06290 100644 --- a/porch/pkg/engine/package.go +++ b/porch/pkg/engine/package.go @@ -24,7 +24,7 @@ import ( ) type PackageFetcher struct { - cad CaDEngine + repoOpener RepositoryOpener referenceResolver ReferenceResolver } @@ -38,7 +38,12 @@ func (p *PackageFetcher) FetchRevision(ctx context.Context, packageRef *api.Pack return nil, fmt.Errorf("cannot find repository %s/%s: %w", namespace, repositoryName, err) } - revisions, err := p.cad.ListPackageRevisions(ctx, &resolved, repository.ListPackageRevisionFilter{KubeObjectName: packageRef.Name}) + repo, err := p.repoOpener.OpenRepository(ctx, &resolved) + if err != nil { + return nil, err + } + + revisions, err := repo.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{KubeObjectName: packageRef.Name}) if err != nil { return nil, err } diff --git a/porch/pkg/git/draft.go b/porch/pkg/git/draft.go index c1ad0af920..3a9757ff42 100644 --- a/porch/pkg/git/draft.go +++ b/porch/pkg/git/draft.go @@ -174,7 +174,10 @@ func (r *gitRepository) closeDraft(ctx context.Context, d *gitPackageDraft) (*gi } if err := d.parent.pushAndCleanup(ctx, refSpecs); err != nil { - return nil, err + // No changes is fine. No need to return an error. + if !errors.Is(err, git.NoErrAlreadyUpToDate) { + return nil, err + } } return &gitPackageRevision{ diff --git a/porch/pkg/git/package.go b/porch/pkg/git/package.go index dbd0d375c9..e4b6c27842 100644 --- a/porch/pkg/git/package.go +++ b/porch/pkg/git/package.go @@ -59,6 +59,10 @@ func (p *gitPackageRevision) KubeObjectName() string { return p.repo.name + "-" + hex.EncodeToString(hash[:]) } +func (p *gitPackageRevision) KubeObjectNamespace() string { + return p.repo.namespace +} + func (p *gitPackageRevision) Key() repository.PackageRevisionKey { return repository.PackageRevisionKey{ Repository: p.repo.name, diff --git a/porch/pkg/meta/fake/memorystore.go b/porch/pkg/meta/fake/memorystore.go new file mode 100644 index 0000000000..3a7a7a7102 --- /dev/null +++ b/porch/pkg/meta/fake/memorystore.go @@ -0,0 +1,101 @@ +// Copyright 2022 Google LLC +// +// 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 fake + +import ( + "context" + + configapi "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1" + "github.com/GoogleContainerTools/kpt/porch/pkg/meta" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" +) + +// MemoryMetadataStore is an in-memory implementation of the MetadataStore interface. It +// means metadata about packagerevisions will be stored in memory, which is useful for testing. +type MemoryMetadataStore struct { + Metas []meta.PackageRevisionMeta +} + +var _ meta.MetadataStore = &MemoryMetadataStore{} + +func (m *MemoryMetadataStore) Get(ctx context.Context, namespacedName types.NamespacedName) (meta.PackageRevisionMeta, error) { + for _, meta := range m.Metas { + if meta.Name == namespacedName.Name && meta.Namespace == namespacedName.Namespace { + return meta, nil + } + } + return meta.PackageRevisionMeta{}, apierrors.NewNotFound( + schema.GroupResource{Group: "config.kpt.dev", Resource: "packagerevisions"}, + namespacedName.Name, + ) +} + +func (m *MemoryMetadataStore) List(ctx context.Context, repo *configapi.Repository) ([]meta.PackageRevisionMeta, error) { + return m.Metas, nil +} + +func (m *MemoryMetadataStore) Create(ctx context.Context, pkgRevMeta meta.PackageRevisionMeta, repo *configapi.Repository) (meta.PackageRevisionMeta, error) { + for _, m := range m.Metas { + if m.Name == pkgRevMeta.Name && m.Namespace == pkgRevMeta.Namespace { + return m, apierrors.NewAlreadyExists( + schema.GroupResource{Group: "config.kpt.dev", Resource: "packagerevisions"}, + m.Name, + ) + } + } + m.Metas = append(m.Metas, pkgRevMeta) + return pkgRevMeta, nil +} + +func (m *MemoryMetadataStore) Update(ctx context.Context, pkgRevMeta meta.PackageRevisionMeta) (meta.PackageRevisionMeta, error) { + i := -1 + for j, m := range m.Metas { + if m.Name == pkgRevMeta.Name && m.Namespace == pkgRevMeta.Namespace { + i = j + } + } + if i < 0 { + return meta.PackageRevisionMeta{}, apierrors.NewNotFound( + schema.GroupResource{Group: "config.porch.kpt.dev", Resource: "packagerevisions"}, + pkgRevMeta.Name, + ) + } + m.Metas[i] = pkgRevMeta + return pkgRevMeta, nil +} + +func (m *MemoryMetadataStore) Delete(ctx context.Context, namespacedName types.NamespacedName) (meta.PackageRevisionMeta, error) { + var metas []meta.PackageRevisionMeta + found := false + var deletedMeta meta.PackageRevisionMeta + for _, m := range m.Metas { + if m.Name == namespacedName.Name && m.Namespace == namespacedName.Namespace { + found = true + deletedMeta = m + } else { + metas = append(metas, m) + } + } + if !found { + return meta.PackageRevisionMeta{}, apierrors.NewNotFound( + schema.GroupResource{Group: "config.kpt.dev", Resource: "packagerevisions"}, + namespacedName.Name, + ) + } + m.Metas = metas + return deletedMeta, nil +} diff --git a/porch/pkg/meta/store.go b/porch/pkg/meta/store.go new file mode 100644 index 0000000000..aa41edcf34 --- /dev/null +++ b/porch/pkg/meta/store.go @@ -0,0 +1,229 @@ +// Copyright 2022 Google LLC +// +// 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 meta + +import ( + "context" + + configapi "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1" + internalapi "github.com/GoogleContainerTools/kpt/porch/internal/api/porchinternal/v1alpha1" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/trace" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var tracer = otel.Tracer("meta") + +const ( + PkgRevisionRepoLabel = "internal.porch.kpt.dev/repository" +) + +// MetadataStore is the store for keeping metadata about PackageRevisions. Typical +// examples of metadata we want to keep is labels, annotations, owner references, and +// finalizers. +type MetadataStore interface { + Get(ctx context.Context, namespacedName types.NamespacedName) (PackageRevisionMeta, error) + List(ctx context.Context, repo *configapi.Repository) ([]PackageRevisionMeta, error) + Create(ctx context.Context, pkgRevMeta PackageRevisionMeta, repo *configapi.Repository) (PackageRevisionMeta, error) + Update(ctx context.Context, pkgRevMeta PackageRevisionMeta) (PackageRevisionMeta, error) + Delete(ctx context.Context, namespacedName types.NamespacedName) (PackageRevisionMeta, error) +} + +// PackageRevisionMeta contains metadata about a specific PackageRevision. The +// PackageRevision is identified by the name and namespace. +type PackageRevisionMeta struct { + Name string + Namespace string + Labels map[string]string + Annotations map[string]string +} + +var _ MetadataStore = &crdMetadataStore{} + +func NewCrdMetadataStore(coreClient client.Client) *crdMetadataStore { + return &crdMetadataStore{ + coreClient: coreClient, + } +} + +// crdMetadataStore is an implementation of the MetadataStore interface that +// stores metadata in a CRD. +type crdMetadataStore struct { + coreClient client.Client +} + +func (c *crdMetadataStore) Get(ctx context.Context, namespacedName types.NamespacedName) (PackageRevisionMeta, error) { + ctx, span := tracer.Start(ctx, "crdMetadataStore::Get", trace.WithAttributes()) + defer span.End() + + var internalPkgRev internalapi.PackageRev + err := c.coreClient.Get(ctx, namespacedName, &internalPkgRev) + if err != nil { + return PackageRevisionMeta{}, err + } + + labels := internalPkgRev.Labels + delete(labels, PkgRevisionRepoLabel) + + return PackageRevisionMeta{ + Name: internalPkgRev.Name, + Namespace: internalPkgRev.Namespace, + Labels: labels, + Annotations: internalPkgRev.Annotations, + }, nil +} + +func (c *crdMetadataStore) List(ctx context.Context, repo *configapi.Repository) ([]PackageRevisionMeta, error) { + ctx, span := tracer.Start(ctx, "crdMetadataStore::List", trace.WithAttributes()) + defer span.End() + + var internalPkgRevList internalapi.PackageRevList + err := c.coreClient.List(ctx, &internalPkgRevList, client.InNamespace(repo.Namespace), client.MatchingLabels(map[string]string{PkgRevisionRepoLabel: repo.Name})) + if err != nil { + return nil, err + } + var pkgRevMetas []PackageRevisionMeta + var names []string + for _, ipr := range internalPkgRevList.Items { + labels := ipr.Labels + delete(labels, PkgRevisionRepoLabel) + pkgRevMetas = append(pkgRevMetas, PackageRevisionMeta{ + Name: ipr.Name, + Namespace: ipr.Namespace, + Labels: labels, + Annotations: ipr.Annotations, + }) + names = append(names, ipr.Name) + } + return pkgRevMetas, nil +} + +func (c *crdMetadataStore) Create(ctx context.Context, pkgRevMeta PackageRevisionMeta, repo *configapi.Repository) (PackageRevisionMeta, error) { + ctx, span := tracer.Start(ctx, "crdMetadataStore::Create", trace.WithAttributes()) + defer span.End() + + labels := pkgRevMeta.Labels + if labels == nil { + labels = make(map[string]string) + } + labels[PkgRevisionRepoLabel] = repo.Name + internalPkgRev := internalapi.PackageRev{ + ObjectMeta: metav1.ObjectMeta{ + Name: pkgRevMeta.Name, + Namespace: pkgRevMeta.Namespace, + Labels: labels, + Annotations: pkgRevMeta.Annotations, + // We probably should make these owner refs point to the PackageRevision CRs instead. + // But we need to make sure that deletion of these are correctly picked up by the + // GC. Currently we delete PackageRevisions through polling of the git/oci repos, and + // that doesn't get picked up by the GC. + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: configapi.RepositoryGVK.GroupVersion().String(), + Kind: configapi.RepositoryGVK.Kind, + Name: repo.Name, + UID: repo.UID, + }, + }, + }, + } + if err := c.coreClient.Create(ctx, &internalPkgRev); err != nil { + if apierrors.IsAlreadyExists(err) { + return c.Update(ctx, pkgRevMeta) + } + return PackageRevisionMeta{}, err + } + return PackageRevisionMeta{ + Name: internalPkgRev.Name, + Namespace: internalPkgRev.Namespace, + Labels: internalPkgRev.Labels, + Annotations: internalPkgRev.Annotations, + }, nil +} + +func (c *crdMetadataStore) Update(ctx context.Context, pkgRevMeta PackageRevisionMeta) (PackageRevisionMeta, error) { + ctx, span := tracer.Start(ctx, "crdMetadataStore::Update", trace.WithAttributes()) + defer span.End() + + var internalPkgRev internalapi.PackageRev + namespacedName := types.NamespacedName{ + Name: pkgRevMeta.Name, + Namespace: pkgRevMeta.Namespace, + } + err := c.coreClient.Get(ctx, namespacedName, &internalPkgRev) + if err != nil { + return PackageRevisionMeta{}, err + } + + var labels map[string]string + if pkgRevMeta.Labels != nil { + labels = pkgRevMeta.Labels + } else { + labels = make(map[string]string) + } + labels[PkgRevisionRepoLabel] = internalPkgRev.Labels[PkgRevisionRepoLabel] + internalPkgRev.Labels = labels + + var annotations map[string]string + if pkgRevMeta.Annotations != nil { + annotations = pkgRevMeta.Annotations + } else { + annotations = make(map[string]string) + } + internalPkgRev.Annotations = annotations + + if err := c.coreClient.Update(ctx, &internalPkgRev); err != nil { + return PackageRevisionMeta{}, err + } + delete(labels, PkgRevisionRepoLabel) + return PackageRevisionMeta{ + Name: pkgRevMeta.Name, + Namespace: pkgRevMeta.Namespace, + Labels: labels, + Annotations: internalPkgRev.Annotations, + }, nil +} + +func (c *crdMetadataStore) Delete(ctx context.Context, namespacedName types.NamespacedName) (PackageRevisionMeta, error) { + ctx, span := tracer.Start(ctx, "crdMetadataStore::Delete", trace.WithAttributes()) + defer span.End() + + var internalPkgRev internalapi.PackageRev + err := c.coreClient.Get(ctx, namespacedName, &internalPkgRev) + if err != nil { + if apierrors.IsNotFound(err) { + return PackageRevisionMeta{}, nil + } + return PackageRevisionMeta{}, err + } + + if err := c.coreClient.Delete(ctx, &internalPkgRev); err != nil { + if apierrors.IsNotFound(err) { + return PackageRevisionMeta{}, nil + } + return PackageRevisionMeta{}, err + } + labels := internalPkgRev.Labels + delete(labels, PkgRevisionRepoLabel) + return PackageRevisionMeta{ + Name: internalPkgRev.Name, + Namespace: internalPkgRev.Namespace, + Labels: labels, + Annotations: internalPkgRev.Annotations, + }, nil +} diff --git a/porch/pkg/oci/oci.go b/porch/pkg/oci/oci.go index 2232bff39c..8936d4d4d2 100644 --- a/porch/pkg/oci/oci.go +++ b/porch/pkg/oci/oci.go @@ -371,6 +371,10 @@ func (p *ociPackageRevision) KubeObjectName() string { return p.parent.name + "-" + hex.EncodeToString(hash[:]) } +func (p *ociPackageRevision) KubeObjectNamespace() string { + return p.parent.namespace +} + func (p *ociPackageRevision) Key() repository.PackageRevisionKey { return repository.PackageRevisionKey{ Repository: p.parent.name, diff --git a/porch/pkg/registry/porch/package.go b/porch/pkg/registry/porch/package.go index 56c39c8a35..b7f9dd2a5e 100644 --- a/porch/pkg/registry/porch/package.go +++ b/porch/pkg/registry/porch/package.go @@ -19,7 +19,7 @@ import ( "fmt" api "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" - "github.com/GoogleContainerTools/kpt/porch/pkg/repository" + "github.com/GoogleContainerTools/kpt/porch/pkg/engine" "go.opentelemetry.io/otel/trace" apierrors "k8s.io/apimachinery/pkg/api/errors" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" @@ -73,7 +73,7 @@ func (r *packages) List(ctx context.Context, options *metainternalversion.ListOp return nil, err } - if err := r.packageCommon.listPackages(ctx, filter, func(p repository.Package) error { + if err := r.packageCommon.listPackages(ctx, filter, func(p *engine.Package) error { item := p.GetPackage() result.Items = append(result.Items, *item) return nil diff --git a/porch/pkg/registry/porch/packagecommon.go b/porch/pkg/registry/porch/packagecommon.go index d96052cacf..a3bd757d06 100644 --- a/porch/pkg/registry/porch/packagecommon.go +++ b/porch/pkg/registry/porch/packagecommon.go @@ -26,6 +26,7 @@ import ( "github.com/GoogleContainerTools/kpt/porch/pkg/repository" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -47,7 +48,7 @@ type packageCommon struct { createStrategy SimpleRESTCreateStrategy } -func (r *packageCommon) listPackageRevisions(ctx context.Context, filter packageRevisionFilter, callback func(p repository.PackageRevision) error) error { +func (r *packageCommon) listPackageRevisions(ctx context.Context, filter packageRevisionFilter, selector labels.Selector, callback func(p *engine.PackageRevision) error) error { var opts []client.ListOption if ns, namespaced := genericapirequest.NamespaceFrom(ctx); namespaced { opts = append(opts, client.InNamespace(ns)) @@ -75,6 +76,11 @@ func (r *packageCommon) listPackageRevisions(ctx context.Context, filter package return err } for _, rev := range revisions { + apiPkgRev := rev.GetPackageRevision() + if selector != nil && !selector.Matches(labels.Set(apiPkgRev.Labels)) { + continue + } + if err := callback(rev); err != nil { return err } @@ -83,7 +89,7 @@ func (r *packageCommon) listPackageRevisions(ctx context.Context, filter package return nil } -func (r *packageCommon) listPackages(ctx context.Context, filter packageFilter, callback func(p repository.Package) error) error { +func (r *packageCommon) listPackages(ctx context.Context, filter packageFilter, callback func(p *engine.Package) error) error { var opts []client.ListOption if ns, namespaced := genericapirequest.NamespaceFrom(ctx); namespaced { opts = append(opts, client.InNamespace(ns)) @@ -151,7 +157,7 @@ func (r *packageCommon) getRepositoryObj(ctx context.Context, repositoryID types return &repositoryObj, nil } -func (r *packageCommon) getPackageRevision(ctx context.Context, name string) (repository.PackageRevision, error) { +func (r *packageCommon) getRepoPkgRev(ctx context.Context, name string) (*engine.PackageRevision, error) { repositoryObj, err := r.getRepositoryObjFromName(ctx, name) if err != nil { return nil, err @@ -169,7 +175,7 @@ func (r *packageCommon) getPackageRevision(ctx context.Context, name string) (re return nil, apierrors.NewNotFound(r.gr, name) } -func (r *packageCommon) getPackage(ctx context.Context, name string) (repository.Package, error) { +func (r *packageCommon) getPackage(ctx context.Context, name string) (*engine.Package, error) { repositoryObj, err := r.getRepositoryObjFromName(ctx, name) if err != nil { return nil, err @@ -200,7 +206,7 @@ func (r *packageCommon) updatePackageRevision(ctx context.Context, name string, // isCreate tracks whether this is an update that creates an object (this happens in server-side apply) isCreate := false - oldPackage, err := r.getPackageRevision(ctx, name) + oldRepoPkgRev, err := r.getRepoPkgRev(ctx, name) if err != nil { if forceAllowCreate && apierrors.IsNotFound(err) { // For server-side apply, we can create the object here @@ -210,12 +216,12 @@ func (r *packageCommon) updatePackageRevision(ctx context.Context, name string, } } - var oldRuntimeObj runtime.Object // We have to be runtime.Object (and not *api.PackageRevision) or else nil-checks fail (because a nil object is not a nil interface) + var oldApiPkgRev runtime.Object // We have to be runtime.Object (and not *api.PackageRevision) or else nil-checks fail (because a nil object is not a nil interface) if !isCreate { - oldRuntimeObj = oldPackage.GetPackageRevision() + oldApiPkgRev = oldRepoPkgRev.GetPackageRevision() } - newRuntimeObj, err := objInfo.UpdatedObject(ctx, oldRuntimeObj) + newRuntimeObj, err := objInfo.UpdatedObject(ctx, oldApiPkgRev) if err != nil { klog.Infof("update failed to construct UpdatedObject: %v", err) return nil, false, err @@ -232,12 +238,12 @@ func (r *packageCommon) updatePackageRevision(ctx context.Context, name string, newRuntimeObj = typed } - if err := r.validateUpdate(ctx, newRuntimeObj, oldRuntimeObj, isCreate, createValidation, + if err := r.validateUpdate(ctx, newRuntimeObj, oldApiPkgRev, isCreate, createValidation, updateValidation, "PackageRevision", name); err != nil { return nil, false, err } - newObj, ok := newRuntimeObj.(*api.PackageRevision) + newApiPkgRev, ok := newRuntimeObj.(*api.PackageRevision) if !ok { return nil, false, apierrors.NewBadRequest(fmt.Sprintf("expected PackageRevision object, got %T", newRuntimeObj)) } @@ -247,7 +253,7 @@ func (r *packageCommon) updatePackageRevision(ctx context.Context, name string, return nil, false, apierrors.NewBadRequest(fmt.Sprintf("invalid name %q", name)) } if isCreate { - repositoryName = newObj.Spec.RepositoryName + repositoryName = newApiPkgRev.Spec.RepositoryName if repositoryName == "" { return nil, false, apierrors.NewBadRequest(fmt.Sprintf("invalid repositoryName %q", name)) } @@ -263,23 +269,22 @@ func (r *packageCommon) updatePackageRevision(ctx context.Context, name string, } if !isCreate { - rev, err := r.cad.UpdatePackageRevision(ctx, &repositoryObj, oldPackage, oldRuntimeObj.(*api.PackageRevision), newObj) + rev, err := r.cad.UpdatePackageRevision(ctx, &repositoryObj, oldRepoPkgRev, oldApiPkgRev.(*api.PackageRevision), newApiPkgRev) if err != nil { return nil, false, apierrors.NewInternalError(err) } - updated := rev.GetPackageRevision() return updated, false, nil } else { - rev, err := r.cad.CreatePackageRevision(ctx, &repositoryObj, newObj) + rev, err := r.cad.CreatePackageRevision(ctx, &repositoryObj, newApiPkgRev) if err != nil { klog.Infof("error creating package: %v", err) return nil, false, apierrors.NewInternalError(err) } + createdApiPkgRev := rev.GetPackageRevision() - created := rev.GetPackageRevision() - return created, true, nil + return createdApiPkgRev, true, nil } } diff --git a/porch/pkg/registry/porch/packagerevision.go b/porch/pkg/registry/porch/packagerevision.go index e41eb3889b..1e74c63c8a 100644 --- a/porch/pkg/registry/porch/packagerevision.go +++ b/porch/pkg/registry/porch/packagerevision.go @@ -19,7 +19,7 @@ import ( "fmt" api "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" - "github.com/GoogleContainerTools/kpt/porch/pkg/repository" + "github.com/GoogleContainerTools/kpt/porch/pkg/engine" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/trace" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -76,7 +76,7 @@ func (r *packageRevisions) List(ctx context.Context, options *metainternalversio return nil, err } - if err := r.packageCommon.listPackageRevisions(ctx, filter, func(p repository.PackageRevision) error { + if err := r.packageCommon.listPackageRevisions(ctx, filter, options.LabelSelector, func(p *engine.PackageRevision) error { item := p.GetPackageRevision() result.Items = append(result.Items, *item) return nil @@ -92,13 +92,14 @@ func (r *packageRevisions) Get(ctx context.Context, name string, options *metav1 ctx, span := tracer.Start(ctx, "packageRevisions::Get", trace.WithAttributes()) defer span.End() - pkg, err := r.getPackageRevision(ctx, name) + repoPkgRev, err := r.getRepoPkgRev(ctx, name) if err != nil { return nil, err } - obj := pkg.GetPackageRevision() - return obj, nil + apiPkgRev := repoPkgRev.GetPackageRevision() + + return apiPkgRev, nil } // Create implements the Creater interface. @@ -111,7 +112,7 @@ func (r *packageRevisions) Create(ctx context.Context, runtimeObject runtime.Obj return nil, apierrors.NewBadRequest("namespace must be specified") } - obj, ok := runtimeObject.(*api.PackageRevision) + newApiPkgRev, ok := runtimeObject.(*api.PackageRevision) if !ok { return nil, apierrors.NewBadRequest(fmt.Sprintf("expected PackageRevision object, got %T", runtimeObject)) } @@ -119,11 +120,11 @@ func (r *packageRevisions) Create(ctx context.Context, runtimeObject runtime.Obj // TODO: Accpept 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 obj.Name != "" { - klog.Warningf("Client provided metadata.name %q", obj.Name) + if newApiPkgRev.Name != "" { + klog.Warningf("Client provided metadata.name %q", newApiPkgRev.Name) } - repositoryName := obj.Spec.RepositoryName + repositoryName := newApiPkgRev.Spec.RepositoryName if repositoryName == "" { return nil, apierrors.NewBadRequest("spec.repositoryName is required") } @@ -135,16 +136,17 @@ func (r *packageRevisions) Create(ctx context.Context, runtimeObject runtime.Obj fieldErrors := r.createStrategy.Validate(ctx, runtimeObject) if len(fieldErrors) > 0 { - return nil, apierrors.NewInvalid(api.SchemeGroupVersion.WithKind("PackageRevision").GroupKind(), obj.Name, fieldErrors) + return nil, apierrors.NewInvalid(api.SchemeGroupVersion.WithKind("PackageRevision").GroupKind(), newApiPkgRev.Name, fieldErrors) } - rev, err := r.cad.CreatePackageRevision(ctx, repositoryObj, obj) + createdRepoPkgRev, err := r.cad.CreatePackageRevision(ctx, repositoryObj, newApiPkgRev) if err != nil { return nil, apierrors.NewInternalError(err) } - created := rev.GetPackageRevision() - return created, nil + createdApiPkgRev := createdRepoPkgRev.GetPackageRevision() + + return createdApiPkgRev, nil } // Update implements the Updater interface. @@ -179,21 +181,22 @@ func (r *packageRevisions) Delete(ctx context.Context, name string, deleteValida return nil, false, apierrors.NewBadRequest("namespace must be specified") } - oldPackage, err := r.packageCommon.getPackageRevision(ctx, name) + repoPkgRev, err := r.packageCommon.getRepoPkgRev(ctx, name) if err != nil { return nil, false, err } - oldObj := oldPackage.GetPackageRevision() - repositoryObj, err := r.packageCommon.validateDelete(ctx, deleteValidation, oldObj, name, ns) + apiPkgRev := repoPkgRev.GetPackageRevision() + + repositoryObj, err := r.packageCommon.validateDelete(ctx, deleteValidation, apiPkgRev, name, ns) if err != nil { return nil, false, err } - if err := r.cad.DeletePackageRevision(ctx, repositoryObj, oldPackage); err != nil { + if err := r.cad.DeletePackageRevision(ctx, repositoryObj, repoPkgRev); err != nil { return nil, false, apierrors.NewInternalError(err) } // TODO: Should we do an async delete? - return oldObj, true, nil + return apiPkgRev, true, nil } diff --git a/porch/pkg/registry/porch/packagerevision_test.go b/porch/pkg/registry/porch/packagerevision_test.go index 09b1a686ce..3de6e31567 100644 --- a/porch/pkg/registry/porch/packagerevision_test.go +++ b/porch/pkg/registry/porch/packagerevision_test.go @@ -15,12 +15,14 @@ package porch import ( + "context" "testing" api "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestUpdateStrategy(t *testing.T) { +func TestUpdateStrategyForLifecycle(t *testing.T) { type testCase struct { old api.PackageRevisionLifecycle valid []api.PackageRevisionLifecycle @@ -47,8 +49,8 @@ func TestUpdateStrategy(t *testing.T) { }, { old: api.PackageRevisionLifecyclePublished, - valid: []api.PackageRevisionLifecycle{}, - invalid: []api.PackageRevisionLifecycle{"", "Wrong", api.PackageRevisionLifecycleDraft, api.PackageRevisionLifecycleProposed, api.PackageRevisionLifecyclePublished}, + valid: []api.PackageRevisionLifecycle{api.PackageRevisionLifecyclePublished}, + invalid: []api.PackageRevisionLifecycle{"", "Wrong", api.PackageRevisionLifecycleDraft, api.PackageRevisionLifecycleProposed}, }, { old: "Wrong", @@ -64,3 +66,144 @@ func TestUpdateStrategy(t *testing.T) { } } } + +func TestUpdateStrategy(t *testing.T) { + s := packageRevisionStrategy{} + + testCases := map[string]struct { + old *api.PackageRevision + new *api.PackageRevision + valid bool + }{ + "spec can be updated for draft": { + old: &api.PackageRevision{ + Spec: api.PackageRevisionSpec{ + Lifecycle: api.PackageRevisionLifecycleDraft, + Tasks: []api.Task{ + { + Type: api.TaskTypeInit, + Init: &api.PackageInitTaskSpec{ + Description: "This is a test", + }, + }, + }, + }, + }, + new: &api.PackageRevision{ + Spec: api.PackageRevisionSpec{ + Lifecycle: api.PackageRevisionLifecycleDraft, + Tasks: []api.Task{ + { + Type: api.TaskTypeInit, + Init: &api.PackageInitTaskSpec{ + Description: "This is a test", + }, + }, + { + Type: api.TaskTypePatch, + Patch: &api.PackagePatchTaskSpec{}, + }, + }, + }, + }, + valid: true, + }, + "spec can not be updated for published": { + old: &api.PackageRevision{ + Spec: api.PackageRevisionSpec{ + Lifecycle: api.PackageRevisionLifecyclePublished, + Tasks: []api.Task{ + { + Type: api.TaskTypeInit, + Init: &api.PackageInitTaskSpec{ + Description: "This is a test", + }, + }, + }, + }, + }, + new: &api.PackageRevision{ + Spec: api.PackageRevisionSpec{ + Lifecycle: api.PackageRevisionLifecyclePublished, + Tasks: []api.Task{ + { + Type: api.TaskTypeInit, + Init: &api.PackageInitTaskSpec{ + Description: "This is a test", + }, + }, + { + Type: api.TaskTypePatch, + Patch: &api.PackagePatchTaskSpec{}, + }, + }, + }, + }, + valid: false, + }, + "labels can be updated for published": { + old: &api.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "foo": "bar", + }, + }, + Spec: api.PackageRevisionSpec{ + Lifecycle: api.PackageRevisionLifecyclePublished, + }, + }, + new: &api.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "bar": "foo", + }, + }, + Spec: api.PackageRevisionSpec{ + Lifecycle: api.PackageRevisionLifecyclePublished, + }, + }, + valid: true, + }, + "annotations can be updated for published": { + old: &api.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "foo": "bar", + }, + }, + Spec: api.PackageRevisionSpec{ + Lifecycle: api.PackageRevisionLifecyclePublished, + }, + }, + new: &api.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "bar": "foo", + }, + }, + Spec: api.PackageRevisionSpec{ + Lifecycle: api.PackageRevisionLifecyclePublished, + }, + }, + valid: true, + }, + } + + for tn := range testCases { + tc := testCases[tn] + t.Run(tn, func(t *testing.T) { + ctx := context.Background() + allErrs := s.ValidateUpdate(ctx, tc.new, tc.old) + + if tc.valid { + if len(allErrs) > 0 { + t.Errorf("Update failed unexpectedly: %v", allErrs.ToAggregate().Error()) + } + } else { + if len(allErrs) == 0 { + t.Error("Update should fail but didn't") + } + } + }) + } +} diff --git a/porch/pkg/registry/porch/packagerevisionresources.go b/porch/pkg/registry/porch/packagerevisionresources.go index 060dab2208..3378db789d 100644 --- a/porch/pkg/registry/porch/packagerevisionresources.go +++ b/porch/pkg/registry/porch/packagerevisionresources.go @@ -17,11 +17,12 @@ package porch import ( "context" "fmt" + "k8s.io/apimachinery/pkg/runtime/schema" api "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1" - "github.com/GoogleContainerTools/kpt/porch/pkg/repository" + "github.com/GoogleContainerTools/kpt/porch/pkg/engine" "go.opentelemetry.io/otel/trace" apierrors "k8s.io/apimachinery/pkg/api/errors" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" @@ -75,12 +76,12 @@ func (r *packageRevisionResources) List(ctx context.Context, options *metaintern return nil, err } - if err := r.packageCommon.listPackageRevisions(ctx, filter, func(p repository.PackageRevision) error { - item, err := p.GetResources(ctx) + if err := r.packageCommon.listPackageRevisions(ctx, filter, options.LabelSelector, func(p *engine.PackageRevision) error { + apiPkgResources, err := p.GetResources(ctx) if err != nil { return err } - result.Items = append(result.Items, *item) + result.Items = append(result.Items, *apiPkgResources) return nil }); err != nil { return nil, err @@ -94,16 +95,16 @@ func (r *packageRevisionResources) Get(ctx context.Context, name string, options ctx, span := tracer.Start(ctx, "packageRevisionResources::Get", trace.WithAttributes()) defer span.End() - pkg, err := r.packageCommon.getPackageRevision(ctx, name) + pkg, err := r.packageCommon.getRepoPkgRev(ctx, name) if err != nil { return nil, err } - obj, err := pkg.GetResources(ctx) + apiPkgResources, err := pkg.GetResources(ctx) if err != nil { return nil, err } - return obj, nil + return apiPkgResources, nil } // Update finds a resource in the storage and updates it. Some implementations @@ -118,18 +119,18 @@ func (r *packageRevisionResources) Update(ctx context.Context, name string, objI return nil, false, apierrors.NewBadRequest("namespace must be specified") } - oldPackage, err := r.packageCommon.getPackageRevision(ctx, name) + oldRepoPkgRev, err := r.packageCommon.getRepoPkgRev(ctx, name) if err != nil { return nil, false, err } - oldObj, err := oldPackage.GetResources(ctx) + oldApiPkgRevResources, err := oldRepoPkgRev.GetResources(ctx) if err != nil { klog.Infof("update failed to retrieve old object: %v", err) return nil, false, err } - newRuntimeObj, err := objInfo.UpdatedObject(ctx, oldObj) + newRuntimeObj, err := objInfo.UpdatedObject(ctx, oldApiPkgRevResources) if err != nil { klog.Infof("update failed to construct UpdatedObject: %v", err) return nil, false, err @@ -140,7 +141,7 @@ func (r *packageRevisionResources) Update(ctx context.Context, name string, objI } if updateValidation != nil { - err := updateValidation(ctx, newObj, oldObj) + err := updateValidation(ctx, newObj, oldApiPkgRevResources) if err != nil { klog.Infof("update failed validation: %v", err) return nil, false, err @@ -161,7 +162,7 @@ func (r *packageRevisionResources) Update(ctx context.Context, name string, objI return nil, false, apierrors.NewInternalError(fmt.Errorf("error getting repository %v: %w", repositoryID, err)) } - rev, err := r.cad.UpdatePackageResources(ctx, &repositoryObj, oldPackage, oldObj, newObj) + rev, err := r.cad.UpdatePackageResources(ctx, &repositoryObj, oldRepoPkgRev, oldApiPkgRevResources, newObj) if err != nil { return nil, false, apierrors.NewInternalError(err) } @@ -170,5 +171,6 @@ func (r *packageRevisionResources) Update(ctx context.Context, name string, objI if err != nil { return nil, false, apierrors.NewInternalError(err) } + return created, false, nil } diff --git a/porch/pkg/registry/porch/packagerevisions_approval.go b/porch/pkg/registry/porch/packagerevisions_approval.go index 3e885bc67d..34ac41abf2 100644 --- a/porch/pkg/registry/porch/packagerevisions_approval.go +++ b/porch/pkg/registry/porch/packagerevisions_approval.go @@ -47,7 +47,7 @@ func (a *packageRevisionsApproval) NamespaceScoped() bool { } func (a *packageRevisionsApproval) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { - pkg, err := a.common.getPackageRevision(ctx, name) + pkg, err := a.common.getRepoPkgRev(ctx, name) if err != nil { return nil, err } diff --git a/porch/pkg/registry/porch/strategy.go b/porch/pkg/registry/porch/strategy.go index 84ca714df6..d2dc8367a7 100644 --- a/porch/pkg/registry/porch/strategy.go +++ b/porch/pkg/registry/porch/strategy.go @@ -20,6 +20,7 @@ import ( "strings" api "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" + "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -38,29 +39,51 @@ func (s packageRevisionStrategy) ValidateUpdate(ctx context.Context, obj, old ru oldRevision := old.(*api.PackageRevision) newRevision := obj.(*api.PackageRevision) - switch lifecycle := oldRevision.Spec.Lifecycle; lifecycle { - case "", api.PackageRevisionLifecycleDraft, api.PackageRevisionLifecycleProposed: + // Verify that the new lifecycle value is valid. + switch lifecycle := newRevision.Spec.Lifecycle; lifecycle { + case "", api.PackageRevisionLifecycleDraft, api.PackageRevisionLifecycleProposed, api.PackageRevisionLifecyclePublished: // valid - default: - allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "lifecycle"), lifecycle, fmt.Sprintf("can only update package with lifecycle value one of %s", + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "lifecycle"), lifecycle, fmt.Sprintf("value can be only updated to %s", strings.Join([]string{ string(api.PackageRevisionLifecycleDraft), string(api.PackageRevisionLifecycleProposed), + string(api.PackageRevisionLifecyclePublished), }, ",")), )) - } - switch lifecycle := newRevision.Spec.Lifecycle; lifecycle { + switch lifecycle := oldRevision.Spec.Lifecycle; lifecycle { case "", api.PackageRevisionLifecycleDraft, api.PackageRevisionLifecycleProposed: - // valid - + // Packages in a draft or proposed state can only be updated to draft or proposed. + newLifecycle := newRevision.Spec.Lifecycle + if !(newLifecycle == api.PackageRevisionLifecycleDraft || + newLifecycle == api.PackageRevisionLifecycleProposed || + newLifecycle == "") { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "lifecycle"), lifecycle, fmt.Sprintf("value can be only updated to %s", + strings.Join([]string{ + string(api.PackageRevisionLifecycleDraft), + string(api.PackageRevisionLifecycleProposed), + }, ",")), + )) + } + case api.PackageRevisionLifecyclePublished: + // We don't allow any updates to the spec for packagerevision that have been published. That includes updates of the lifecycle. But + // we allow updates to metadata and status. + if !equality.Semantic.DeepEqual(oldRevision.Spec, newRevision.Spec) { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec"), newRevision.Spec, fmt.Sprintf("spec can only update package with lifecycle value one of %s", + strings.Join([]string{ + string(api.PackageRevisionLifecycleDraft), + string(api.PackageRevisionLifecycleProposed), + }, ",")), + )) + } default: - allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "lifecycle"), lifecycle, fmt.Sprintf("value can be only updated to %s", + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "lifecycle"), lifecycle, fmt.Sprintf("can only update package with lifecycle value one of %s", strings.Join([]string{ string(api.PackageRevisionLifecycleDraft), string(api.PackageRevisionLifecycleProposed), + string(api.PackageRevisionLifecyclePublished), }, ",")), )) } diff --git a/porch/pkg/registry/porch/watch.go b/porch/pkg/registry/porch/watch.go index 8500c48201..a03a20729d 100644 --- a/porch/pkg/registry/porch/watch.go +++ b/porch/pkg/registry/porch/watch.go @@ -19,9 +19,11 @@ import ( "fmt" "sync" + "github.com/GoogleContainerTools/kpt/porch/pkg/engine" "github.com/GoogleContainerTools/kpt/porch/pkg/repository" "go.opentelemetry.io/otel/trace" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/watch" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/klog/v2" @@ -56,7 +58,7 @@ func (r *packageRevisionResources) Watch(ctx context.Context, options *metainter resultChan: make(chan watch.Event, 64), } - go w.listAndWatch(ctx, r, filter) + go w.listAndWatch(ctx, r, filter, options.LabelSelector) return w, nil } @@ -89,8 +91,8 @@ func (w *watcher) ResultChan() <-chan watch.Event { // listAndWatch implements watch by doing a list, then sending any observed changes. // This is not a compliant implementation of watch, but it is a good-enough start for most controllers. // One trick is that we start the watch _before_ we perform the list, so we don't miss changes that happen immediately after the list. -func (w *watcher) listAndWatch(ctx context.Context, r *packageRevisionResources, filter packageRevisionFilter) { - if err := w.listAndWatchInner(ctx, r, filter); err != nil { +func (w *watcher) listAndWatch(ctx context.Context, r *packageRevisionResources, filter packageRevisionFilter, selector labels.Selector) { + if err := w.listAndWatchInner(ctx, r, filter, selector); err != nil { // TODO: We need to populate the object on this error klog.Warningf("sending error to watch stream") ev := watch.Event{ @@ -102,7 +104,7 @@ func (w *watcher) listAndWatch(ctx context.Context, r *packageRevisionResources, close(w.resultChan) } -func (w *watcher) listAndWatchInner(ctx context.Context, r *packageRevisionResources, filter packageRevisionFilter) error { +func (w *watcher) listAndWatchInner(ctx context.Context, r *packageRevisionResources, filter packageRevisionFilter, selector labels.Selector) error { errorResult := make(chan error, 4) done := false @@ -131,7 +133,7 @@ func (w *watcher) listAndWatchInner(ctx context.Context, r *packageRevisionResou } // TODO: Only if rv == 0? - if err := r.packageCommon.listPackageRevisions(ctx, filter, func(p repository.PackageRevision) error { + if err := r.packageCommon.listPackageRevisions(ctx, filter, selector, func(p *engine.PackageRevision) error { obj, err := p.GetResources(ctx) if err != nil { done = true diff --git a/porch/pkg/repository/repository.go b/porch/pkg/repository/repository.go index 77902b508c..77850ca15b 100644 --- a/porch/pkg/repository/repository.go +++ b/porch/pkg/repository/repository.go @@ -53,6 +53,8 @@ type PackageRevision interface { // More "readable" values are returned by Key() KubeObjectName() string + KubeObjectNamespace() string + // Key returns the "primary key" of the package. Key() PackageRevisionKey diff --git a/porch/scripts/create-deployment-blueprint.sh b/porch/scripts/create-deployment-blueprint.sh index 6921eff993..56280754eb 100755 --- a/porch/scripts/create-deployment-blueprint.sh +++ b/porch/scripts/create-deployment-blueprint.sh @@ -188,6 +188,8 @@ function main() { # Repository CRD cp "./api/porchconfig/v1alpha1/config.porch.kpt.dev_repositories.yaml" \ "${DESTINATION}/0-repositories.yaml" + cp "./internal/api/porchinternal/v1alpha1/config.porch.kpt.dev_packagerevs.yaml" \ + "${DESTINATION}/0-packagerevs.yaml" # Porch Deployment Config cp ${PORCH_DIR}/deployments/porch/*.yaml "${PORCH_DIR}/deployments/porch/Kptfile" "${DESTINATION}" diff --git a/porch/test/e2e/e2e_test.go b/porch/test/e2e/e2e_test.go index 62fcc83f69..c07db01b33 100644 --- a/porch/test/e2e/e2e_test.go +++ b/porch/test/e2e/e2e_test.go @@ -27,6 +27,7 @@ import ( kptfilev1 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1" porchapi "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" configapi "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1" + internalapi "github.com/GoogleContainerTools/kpt/porch/internal/api/porchinternal/v1alpha1" "github.com/GoogleContainerTools/kpt/porch/pkg/repository" "github.com/google/go-cmp/cmp" coreapi "k8s.io/api/core/v1" @@ -920,6 +921,15 @@ func (t *PorchSuite) TestPackageUpdate(ctx context.Context) { revisionResources.Spec.Resources["config-map.yaml"] = string(cm) t.UpdateF(ctx, &revisionResources) + var newrr porchapi.PackageRevisionResources + t.GetF(ctx, client.ObjectKey{ + Namespace: t.namespace, + Name: pr.Name, + }, &newrr) + + by, _ := yaml.Marshal(&newrr) + t.Logf("PRR: %s", string(by)) + t.GetF(ctx, client.ObjectKey{ Namespace: t.namespace, Name: pr.Name, @@ -1482,6 +1492,197 @@ func (t *PorchSuite) TestRepositoryError(ctx context.Context) { } } +func (t *PorchSuite) TestNewPackageRevisionLabels(ctx context.Context) { + const ( + repository = "pkg-rev-labels" + labelKey1 = "kpt.dev/label" + labelVal1 = "foo" + labelKey2 = "kpt.dev/other-label" + labelVal2 = "bar" + annoKey1 = "kpt.dev/anno" + annoVal1 = "foo" + annoKey2 = "kpt.dev/other-anno" + annoVal2 = "bar" + ) + + t.registerMainGitRepositoryF(ctx, repository) + + // Create a package with labels and annotations. + pr := porchapi.PackageRevision{ + TypeMeta: metav1.TypeMeta{ + Kind: "PackageRevision", + APIVersion: porchapi.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: t.namespace, + Labels: map[string]string{ + labelKey1: labelVal1, + }, + Annotations: map[string]string{ + annoKey1: annoVal1, + annoKey2: annoVal2, + }, + }, + Spec: porchapi.PackageRevisionSpec{ + PackageName: "new-package", + Revision: "v1", + RepositoryName: repository, + Tasks: []porchapi.Task{ + { + Type: porchapi.TaskTypeInit, + Init: &porchapi.PackageInitTaskSpec{ + Description: "this is a test", + }, + }, + }, + }, + } + t.CreateF(ctx, &pr) + t.validateLabelsAndAnnos(ctx, pr.Name, + map[string]string{ + labelKey1: labelVal1, + }, + map[string]string{ + annoKey1: annoVal1, + annoKey2: annoVal2, + }, + ) + + // Propose the package. + pr.Spec.Lifecycle = porchapi.PackageRevisionLifecycleProposed + t.UpdateF(ctx, &pr) + t.validateLabelsAndAnnos(ctx, pr.Name, + map[string]string{ + labelKey1: labelVal1, + }, + map[string]string{ + annoKey1: annoVal1, + annoKey2: annoVal2, + }, + ) + + // Approve the package + pr.Spec.Lifecycle = porchapi.PackageRevisionLifecyclePublished + _ = t.UpdateApprovalF(ctx, &pr, metav1.UpdateOptions{}) + t.validateLabelsAndAnnos(ctx, pr.Name, + map[string]string{ + labelKey1: labelVal1, + porchapi.LatestPackageRevisionKey: porchapi.LatestPackageRevisionValue, + }, + map[string]string{ + annoKey1: annoVal1, + annoKey2: annoVal2, + }, + ) + + // Update the labels and annotations on the approved package. + delete(pr.ObjectMeta.Labels, labelKey1) + pr.ObjectMeta.Labels[labelKey2] = labelVal2 + delete(pr.ObjectMeta.Annotations, annoKey2) + t.UpdateF(ctx, &pr) + t.validateLabelsAndAnnos(ctx, pr.Name, + map[string]string{ + labelKey2: labelVal2, + porchapi.LatestPackageRevisionKey: porchapi.LatestPackageRevisionValue, + }, + map[string]string{ + annoKey1: annoVal1, + }, + ) + + // Create PackageRevision from upstream repo. Labels and annotations should + // not be retained from upstream. + clonedPr := porchapi.PackageRevision{ + TypeMeta: metav1.TypeMeta{ + Kind: "PackageRevision", + APIVersion: porchapi.SchemeGroupVersion.Identifier(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: t.namespace, + }, + Spec: porchapi.PackageRevisionSpec{ + PackageName: "cloned-package", + Revision: "v1", + RepositoryName: repository, + Tasks: []porchapi.Task{ + { + Type: porchapi.TaskTypeClone, + Clone: &porchapi.PackageCloneTaskSpec{ + Upstream: porchapi.UpstreamPackage{ + UpstreamRef: &porchapi.PackageRevisionRef{ + Name: pr.Name, // Package to be cloned + }, + }, + }, + }, + }, + }, + } + t.CreateF(ctx, &clonedPr) + t.validateLabelsAndAnnos(ctx, clonedPr.Name, + map[string]string{}, + map[string]string{}, + ) +} + +func (t *PorchSuite) TestRegisteredPackageRevisionLabels(ctx context.Context) { + const ( + labelKey = "kpt.dev/label" + labelVal = "foo" + annoKey = "kpt.dev/anno" + annoVal = "foo" + ) + + t.registerGitRepositoryF(ctx, testBlueprintsRepo, "test-blueprints", "") + + var list porchapi.PackageRevisionList + t.ListE(ctx, &list, client.InNamespace(t.namespace)) + + basens := MustFindPackageRevision(t.T, &list, repository.PackageRevisionKey{Repository: "test-blueprints", Package: "basens", Revision: "v1"}) + if basens.ObjectMeta.Labels == nil { + basens.ObjectMeta.Labels = make(map[string]string) + } + basens.ObjectMeta.Labels[labelKey] = labelVal + if basens.ObjectMeta.Annotations == nil { + basens.ObjectMeta.Annotations = make(map[string]string) + } + basens.ObjectMeta.Annotations[annoKey] = annoVal + t.UpdateF(ctx, basens) + + t.validateLabelsAndAnnos(ctx, basens.Name, + map[string]string{ + labelKey: labelVal, + }, + map[string]string{ + annoKey: annoVal, + }, + ) +} + +func (t *PorchSuite) validateLabelsAndAnnos(ctx context.Context, name string, labels, annos map[string]string) { + var pr porchapi.PackageRevision + t.GetF(ctx, client.ObjectKey{ + Namespace: t.namespace, + Name: name, + }, &pr) + + actualLabels := pr.ObjectMeta.Labels + actualAnnos := pr.ObjectMeta.Annotations + + // Make this check to handle empty vs nil maps + if !(len(labels) == 0 && len(actualLabels) == 0) { + if diff := cmp.Diff(actualLabels, labels); diff != "" { + t.Errorf("Unexpected result (-want, +got): %s", diff) + } + } + + if !(len(annos) == 0 && len(actualAnnos) == 0) { + if diff := cmp.Diff(actualAnnos, annos); diff != "" { + t.Errorf("Unexpected result (-want, +got): %s", diff) + } + } +} + func (t *PorchSuite) registerGitRepositoryF(ctx context.Context, repo, name, directory string) { t.CreateF(ctx, &configapi.Repository{ TypeMeta: metav1.TypeMeta{ @@ -1586,6 +1787,8 @@ func (t *PorchSuite) registerMainGitRepositoryF(ctx context.Context, name string Namespace: t.namespace, }, }) + t.waitUntilRepositoryDeleted(ctx, name, t.namespace) + t.waitUntilAllPackagesDeleted(ctx, name) }) } @@ -1677,3 +1880,55 @@ func (t *PorchSuite) waitUntilRepositoryReady(ctx context.Context, name, namespa t.Errorf("Repository not ready after wait: %v", innerErr) } } + +func (t *PorchSuite) waitUntilRepositoryDeleted(ctx context.Context, name, namespace string) { + err := wait.PollImmediateWithContext(ctx, time.Second, 20*time.Second, func(ctx context.Context) (done bool, err error) { + var repo configapi.Repository + nn := types.NamespacedName{ + Name: name, + Namespace: namespace, + } + if err := t.client.Get(ctx, nn, &repo); err != nil { + if apierrors.IsNotFound(err) { + return true, nil + } + return false, nil + } + return false, nil + }) + if err != nil { + t.Fatalf("Repository %s/%s not deleted", namespace, name) + } +} + +func (t *PorchSuite) waitUntilAllPackagesDeleted(ctx context.Context, repoName string) { + err := wait.PollImmediateWithContext(ctx, time.Second, 20*time.Second, func(ctx context.Context) (done bool, err error) { + var pkgRevList porchapi.PackageRevisionList + if err := t.client.List(ctx, &pkgRevList); err != nil { + t.Logf("error listing packages: %v", err) + return false, nil + } + for _, pkgRev := range pkgRevList.Items { + if strings.HasPrefix(fmt.Sprintf("%s-", pkgRev.Name), repoName) { + t.Logf("Found package %s from repo %s", pkgRev.Name, repoName) + return false, nil + } + } + + var internalPkgRevList internalapi.PackageRevList + if err := t.client.List(ctx, &internalPkgRevList); err != nil { + t.Logf("error list internal packages: %v", err) + return false, nil + } + for _, internalPkgRev := range internalPkgRevList.Items { + if strings.HasPrefix(fmt.Sprintf("%s-", internalPkgRev.Name), repoName) { + t.Logf("Found internalPkg %s from repo %s", internalPkgRev.Name, repoName) + return false, nil + } + } + return true, nil + }) + if err != nil { + t.Fatalf("Packages from repo %s still remains", repoName) + } +} diff --git a/porch/test/e2e/suite.go b/porch/test/e2e/suite.go index fd2efecaff..7548d53c81 100644 --- a/porch/test/e2e/suite.go +++ b/porch/test/e2e/suite.go @@ -33,6 +33,7 @@ import ( porchclient "github.com/GoogleContainerTools/kpt/porch/api/generated/clientset/versioned" porchapi "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" configapi "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1" + internalapi "github.com/GoogleContainerTools/kpt/porch/internal/api/porchinternal/v1alpha1" "github.com/GoogleContainerTools/kpt/porch/pkg/git" "github.com/GoogleContainerTools/kpt/porch/pkg/repository" gogit "github.com/go-git/go-git/v5" @@ -292,6 +293,7 @@ func createClientScheme(t *testing.T) *runtime.Scheme { for _, api := range (runtime.SchemeBuilder{ porchapi.AddToScheme, + internalapi.AddToScheme, configapi.AddToScheme, coreapi.AddToScheme, aggregatorv1.AddToScheme,