From 6df205b0a7148bc2a2f39bf503a8a34b1d21a1b6 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Fri, 4 Oct 2024 13:12:25 +0100 Subject: [PATCH 1/3] Remove Function CRD from porchconfig and Repo --- .../config.porch.kpt.dev_functions.yaml | 121 ------------ .../config.porch.kpt.dev_repositories.yaml | 37 +--- api/porchconfig/v1alpha1/groupversion_info.go | 8 +- api/porchconfig/v1alpha1/types.go | 21 +-- api/porchconfig/v1alpha1/types_functions.go | 107 ----------- .../v1alpha1/zz_generated.deepcopy.go | 148 --------------- pkg/cache/cache.go | 2 - pkg/cache/memory/cache_test.go | 1 - pkg/cache/memory/repository.go | 44 ----- pkg/cli/commands/repo/reg/command.go | 1 - pkg/engine/engine.go | 37 ---- pkg/oci/function.go | 174 ------------------ pkg/oci/oci.go | 145 +-------------- pkg/registry/porch/functions.go | 155 ---------------- pkg/registry/porch/storage.go | 7 - pkg/repository/repository.go | 13 -- .../cli/testdata/repo-register/config.yaml | 10 +- .../cli/testdata/rpkg-init-deploy/config.yaml | 4 +- test/e2e/e2e_test.go | 74 -------- test/e2e/suite_utils.go | 7 - 20 files changed, 12 insertions(+), 1104 deletions(-) delete mode 100644 api/porchconfig/v1alpha1/config.porch.kpt.dev_functions.yaml delete mode 100644 api/porchconfig/v1alpha1/types_functions.go delete mode 100644 pkg/oci/function.go delete mode 100644 pkg/registry/porch/functions.go diff --git a/api/porchconfig/v1alpha1/config.porch.kpt.dev_functions.yaml b/api/porchconfig/v1alpha1/config.porch.kpt.dev_functions.yaml deleted file mode 100644 index cfd3bbba..00000000 --- a/api/porchconfig/v1alpha1/config.porch.kpt.dev_functions.yaml +++ /dev/null @@ -1,121 +0,0 @@ ---- -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - annotations: - controller-gen.kubebuilder.io/version: v0.16.1 - name: functions.config.porch.kpt.dev -spec: - group: config.porch.kpt.dev - names: - kind: Function - listKind: FunctionList - plural: functions - singular: function - scope: Namespaced - versions: - - name: v1alpha1 - schema: - openAPIV3Schema: - description: |- - Function represents a kpt function discovered in a repository - Function resources are created automatically by discovery in a registered Repository. - Function resource names will be computed as : - to ensure uniqueness of names, and will follow formatting of - [DNS Subdomain Names](https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names). - 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: FunctionSpec defines the desired state of a Function - properties: - description: - description: Description is a short description of the function. - type: string - documentationUrl: - description: '`DocumentationUrl specifies the URL of comprehensive - function documentation`' - type: string - functionConfigs: - items: - description: |- - FunctionConfig specifies all the valid types of the function config for this function. - If unspecified, defaults to v1/ConfigMap. For example, function `set-namespace` accepts both `ConfigMap` and `SetNamespace` - 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 - requiredFields: - description: |- - Experimental: requiredFields tells necessary fields and is aimed to help users write the FunctionConfig. - Otherwise, users can get the required fields info from the function evaluation error message. - items: - type: string - type: array - type: object - type: array - functionTypes: - description: FunctionType specifies the function types (mutator, validator - or/and others). - items: - type: string - type: array - image: - description: Image specifies the function image, such as 'gcr.io/kpt-fn/gatekeeper:v0.2'. - type: string - keywords: - description: Keywords are used as filters to provide correlation in - function discovery. - items: - type: string - type: array - repositoryRef: - description: RepositoryRef references the repository in which the - function is located. - properties: - name: - description: Name of the Repository resource referenced. - type: string - required: - - name - type: object - required: - - description - - image - - repositoryRef - type: object - status: - description: FunctionStatus defines the observed state of Function - type: object - type: object - served: true - storage: true - subresources: - status: {} diff --git a/api/porchconfig/v1alpha1/config.porch.kpt.dev_repositories.yaml b/api/porchconfig/v1alpha1/config.porch.kpt.dev_repositories.yaml index 669690e9..cc6905aa 100644 --- a/api/porchconfig/v1alpha1/config.porch.kpt.dev_repositories.yaml +++ b/api/porchconfig/v1alpha1/config.porch.kpt.dev_repositories.yaml @@ -18,9 +18,6 @@ spec: - jsonPath: .spec.type name: Type type: string - - jsonPath: .spec.content - name: Content - type: string - jsonPath: .spec.deployment name: Deployment type: boolean @@ -59,10 +56,6 @@ spec: Notes: - deployment repository - in KRM API ConfigSync would be configured directly? (or via this API) properties: - content: - description: Content stored in the repository (i.e. Function, Package - - the literal values correspond to the API resource names). - type: string deployment: description: The repository is a deployment repository; final packages in this repository are deployment ready. @@ -122,22 +115,9 @@ spec: type: string description: '`ConfigMap` specifies the function config (https://kpt.dev/reference/cli/fn/eval/).' type: object - functionRef: - description: '`FunctionRef` specifies the function by reference - to a Function resource. Mutually exclusive with `Image`.' - properties: - name: - description: '`Name` is the name of the `Function` resource - referenced. The resource is expected to be within the - same namespace.' - type: string - required: - - name - type: object image: description: '`Image` specifies the function image, such as - `gcr.io/kpt-fn/gatekeeper:v0.2`. Use of `Image` is mutually - exclusive with `FunctionRef`.' + `gcr.io/kpt-fn/gatekeeper:v0.2`.' type: string type: object type: array @@ -262,22 +242,9 @@ spec: type: string description: '`ConfigMap` specifies the function config (https://kpt.dev/reference/cli/fn/eval/).' type: object - functionRef: - description: '`FunctionRef` specifies the function by reference - to a Function resource. Mutually exclusive with `Image`.' - properties: - name: - description: '`Name` is the name of the `Function` resource - referenced. The resource is expected to be within the - same namespace.' - type: string - required: - - name - type: object image: description: '`Image` specifies the function image, such as - `gcr.io/kpt-fn/gatekeeper:v0.2`. Use of `Image` is mutually - exclusive with `FunctionRef`.' + `gcr.io/kpt-fn/gatekeeper:v0.2`.' type: string type: object type: array diff --git a/api/porchconfig/v1alpha1/groupversion_info.go b/api/porchconfig/v1alpha1/groupversion_info.go index d047e4bb..ba91546c 100644 --- a/api/porchconfig/v1alpha1/groupversion_info.go +++ b/api/porchconfig/v1alpha1/groupversion_info.go @@ -37,13 +37,7 @@ var ( objects: []runtime.Object{&Repository{}, &RepositoryList{}}, } - TypeFunction = TypeInfo{ - Kind: "Function", - Resource: GroupVersion.WithResource("functions"), - objects: []runtime.Object{&Function{}, &FunctionList{}}, - } - - AllKinds = []TypeInfo{TypeRepository, TypeFunction} + AllKinds = []TypeInfo{TypeRepository} ) //+kubebuilder:object:generate=false diff --git a/api/porchconfig/v1alpha1/types.go b/api/porchconfig/v1alpha1/types.go index 0eba861f..9183465b 100644 --- a/api/porchconfig/v1alpha1/types.go +++ b/api/porchconfig/v1alpha1/types.go @@ -22,7 +22,6 @@ import ( //+kubebuilder:subresource:status //+kubebuilder:resource:path=repositories,singular=repository //+kubebuilder:printcolumn:name="Type",type=string,JSONPath=`.spec.type` -//+kubebuilder:printcolumn:name="Content",type=string,JSONPath=`.spec.content` //+kubebuilder:printcolumn:name="Deployment",type=boolean,JSONPath=`.spec.deployment` //+kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=='Ready')].status` //+kubebuilder:printcolumn:name="Address",type=string,JSONPath=`.spec['git','oci']['repo','registry']` @@ -43,13 +42,6 @@ const ( RepositoryTypeOCI RepositoryType = "oci" ) -type RepositoryContent string - -const ( - RepositoryContentFunction RepositoryContent = "Function" - RepositoryContentPackage RepositoryContent = "Package" -) - // RepositorySpec defines the desired state of Repository // // Notes: @@ -61,9 +53,6 @@ type RepositorySpec struct { Deployment bool `json:"deployment,omitempty"` // Type of the repository (i.e. git, OCI) Type RepositoryType `json:"type,omitempty"` - // Content stored in the repository (i.e. Function, Package - the literal values correspond to the API resource names). - // TODO: support repository with mixed content? - Content RepositoryContent `json:"content,omitempty"` // Git repository details. Required if `type` is `git`. Ignored if `type` is not `git`. Git *GitRepository `json:"git,omitempty"` // OCI repository details. Required if `type` is `oci`. Ignored if `type` is not `oci`. @@ -138,20 +127,12 @@ type SecretRef struct { } type FunctionEval struct { - // `Image` specifies the function image, such as `gcr.io/kpt-fn/gatekeeper:v0.2`. Use of `Image` is mutually exclusive with `FunctionRef`. + // `Image` specifies the function image, such as `gcr.io/kpt-fn/gatekeeper:v0.2`. Image string `json:"image,omitempty"` - // `FunctionRef` specifies the function by reference to a Function resource. Mutually exclusive with `Image`. - FunctionRef *FunctionRef `json:"functionRef,omitempty"` // `ConfigMap` specifies the function config (https://kpt.dev/reference/cli/fn/eval/). ConfigMap map[string]string `json:"configMap,omitempty"` } -// `FunctionRef` is a reference to a `Function` resource. -type FunctionRef struct { - // `Name` is the name of the `Function` resource referenced. The resource is expected to be within the same namespace. - Name string `json:"name"` -} - const ( // Type of the Repository condition. RepositoryReady = "Ready" diff --git a/api/porchconfig/v1alpha1/types_functions.go b/api/porchconfig/v1alpha1/types_functions.go deleted file mode 100644 index 896b4e4b..00000000 --- a/api/porchconfig/v1alpha1/types_functions.go +++ /dev/null @@ -1,107 +0,0 @@ -// Copyright 2022 The kpt and Nephio Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package v1alpha1 - -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - -//+kubebuilder:object:root=true -//+kubebuilder:subresource:status -//+kubebuilder:resource:path=functions,singular=function - -// Function represents a kpt function discovered in a repository -// Function resources are created automatically by discovery in a registered Repository. -// Function resource names will be computed as : -// to ensure uniqueness of names, and will follow formatting of -// [DNS Subdomain Names](https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names). -type Function struct { - metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` - - Spec FunctionSpec `json:"spec,omitempty"` - Status FunctionStatus `json:"status,omitempty"` -} - -//+kubebuilder:object:root=true - -// FunctionList -type FunctionList struct { - metav1.TypeMeta `json:",inline"` - metav1.ListMeta `json:"metadata,omitempty"` - - Items []Function `json:"items"` -} - -type FunctionType string - -const ( - FunctionTypeValidator FunctionType = "validator" - FunctionTypeMutator FunctionType = "mutator" -) - -// FunctionSpec defines the desired state of a Function -type FunctionSpec struct { - // Image specifies the function image, such as 'gcr.io/kpt-fn/gatekeeper:v0.2'. - Image string `json:"image"` - - // RepositoryRef references the repository in which the function is located. - RepositoryRef RepositoryRef `json:"repositoryRef"` - - // FunctionType specifies the function types (mutator, validator or/and others). - FunctionTypes []FunctionType `json:"functionTypes,omitempty"` - - FunctionConfigs []FunctionConfig `json:"functionConfigs,omitempty"` - - // Keywords are used as filters to provide correlation in function discovery. - Keywords []string `json:"keywords,omitempty"` - - // Description is a short description of the function. - Description string `json:"description"` - - // `DocumentationUrl specifies the URL of comprehensive function documentation` - DocumentationUrl string `json:"documentationUrl,omitempty"` - - // InputTypes specifies to which input KRM types the function applies. Specified as Group Version Kind. - // For example: - // - // inputTypes: - // - kind: RoleBinding - // # If version is unspecified, applies to all versions - // apiVersion: rbac.authorization.k8s.io - // - kind: ClusterRoleBinding - // apiVersion: rbac.authorization.k8s.io/v1 - // InputTypes []metav1.TypeMeta - - // OutputTypes specifies types of any KRM resources the function creates - // For example: - // - // outputTypes: - // - kind: ConfigMap - // apiVersion: v1 - // OutputTypes []metav1.TypeMeta - -} - -// FunctionConfig specifies all the valid types of the function config for this function. -// If unspecified, defaults to v1/ConfigMap. For example, function `set-namespace` accepts both `ConfigMap` and `SetNamespace` -type FunctionConfig struct { - metav1.TypeMeta `json:",inline"` - // Experimental: requiredFields tells necessary fields and is aimed to help users write the FunctionConfig. - // Otherwise, users can get the required fields info from the function evaluation error message. - RequiredFields []string `json:"requiredFields,omitempty"` -} - -// FunctionStatus defines the observed state of Function -type FunctionStatus struct { -} diff --git a/api/porchconfig/v1alpha1/zz_generated.deepcopy.go b/api/porchconfig/v1alpha1/zz_generated.deepcopy.go index 8d45dc7c..d8d641b0 100644 --- a/api/porchconfig/v1alpha1/zz_generated.deepcopy.go +++ b/api/porchconfig/v1alpha1/zz_generated.deepcopy.go @@ -23,62 +23,9 @@ 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 *Function) DeepCopyInto(out *Function) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Function. -func (in *Function) DeepCopy() *Function { - if in == nil { - return nil - } - out := new(Function) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *Function) 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 *FunctionConfig) DeepCopyInto(out *FunctionConfig) { - *out = *in - out.TypeMeta = in.TypeMeta - if in.RequiredFields != nil { - in, out := &in.RequiredFields, &out.RequiredFields - *out = make([]string, len(*in)) - copy(*out, *in) - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FunctionConfig. -func (in *FunctionConfig) DeepCopy() *FunctionConfig { - if in == nil { - return nil - } - out := new(FunctionConfig) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FunctionEval) DeepCopyInto(out *FunctionEval) { *out = *in - if in.FunctionRef != nil { - in, out := &in.FunctionRef, &out.FunctionRef - *out = new(FunctionRef) - **out = **in - } if in.ConfigMap != nil { in, out := &in.ConfigMap, &out.ConfigMap *out = make(map[string]string, len(*in)) @@ -98,101 +45,6 @@ func (in *FunctionEval) DeepCopy() *FunctionEval { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *FunctionList) DeepCopyInto(out *FunctionList) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ListMeta.DeepCopyInto(&out.ListMeta) - if in.Items != nil { - in, out := &in.Items, &out.Items - *out = make([]Function, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FunctionList. -func (in *FunctionList) DeepCopy() *FunctionList { - if in == nil { - return nil - } - out := new(FunctionList) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *FunctionList) 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 *FunctionRef) DeepCopyInto(out *FunctionRef) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FunctionRef. -func (in *FunctionRef) DeepCopy() *FunctionRef { - if in == nil { - return nil - } - out := new(FunctionRef) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *FunctionSpec) DeepCopyInto(out *FunctionSpec) { - *out = *in - out.RepositoryRef = in.RepositoryRef - if in.FunctionTypes != nil { - in, out := &in.FunctionTypes, &out.FunctionTypes - *out = make([]FunctionType, len(*in)) - copy(*out, *in) - } - if in.FunctionConfigs != nil { - in, out := &in.FunctionConfigs, &out.FunctionConfigs - *out = make([]FunctionConfig, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } - if in.Keywords != nil { - in, out := &in.Keywords, &out.Keywords - *out = make([]string, len(*in)) - copy(*out, *in) - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FunctionSpec. -func (in *FunctionSpec) DeepCopy() *FunctionSpec { - if in == nil { - return nil - } - out := new(FunctionSpec) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *FunctionStatus) DeepCopyInto(out *FunctionStatus) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FunctionStatus. -func (in *FunctionStatus) DeepCopy() *FunctionStatus { - if in == nil { - return nil - } - out := new(FunctionStatus) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GitRepository) DeepCopyInto(out *GitRepository) { *out = *in diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index 11b61595..84e9b97c 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -14,8 +14,6 @@ type Cache interface { type CachedRepository interface { repository.Repository - // TODO: Remove this once https://github.com/nephio-project/porch/pull/119 is merged - repository.FunctionRepository RefreshCache(ctx context.Context) error } diff --git a/pkg/cache/memory/cache_test.go b/pkg/cache/memory/cache_test.go index 2133fe8a..e419075e 100644 --- a/pkg/cache/memory/cache_test.go +++ b/pkg/cache/memory/cache_test.go @@ -153,7 +153,6 @@ func openRepositoryFromArchive(t *testing.T, ctx context.Context, testPath, name Spec: v1alpha1.RepositorySpec{ Deployment: false, Type: v1alpha1.RepositoryTypeGit, - Content: v1alpha1.RepositoryContentPackage, Git: &v1alpha1.GitRepository{ Repo: address, }, diff --git a/pkg/cache/memory/repository.go b/pkg/cache/memory/repository.go index 4e02573e..959182a9 100644 --- a/pkg/cache/memory/repository.go +++ b/pkg/cache/memory/repository.go @@ -44,7 +44,6 @@ var tracer = otel.Tracer("cache") var _ repository.Repository = &cachedRepository{} var _ cache.CachedRepository = &cachedRepository{} -var _ repository.FunctionRepository = &cachedRepository{} type cachedRepository struct { id string @@ -59,9 +58,6 @@ type cachedRepository struct { mutex sync.Mutex cachedPackageRevisions map[repository.PackageRevisionKey]*cachedPackageRevision cachedPackages map[repository.PackageKey]*cachedPackage - - // TODO: Currently we support repositories with homogenous content (only packages xor functions). Model this more optimally? - cachedFunctions []repository.Function // Error encountered on repository refresh by the refresh goroutine. // This is returned back by the cache to the background goroutine when it calls periodicall to resync repositories. refreshRevisionsError error @@ -109,14 +105,6 @@ func (r *cachedRepository) ListPackageRevisions(ctx context.Context, filter repo return packages, nil } -func (r *cachedRepository) ListFunctions(ctx context.Context) ([]repository.Function, error) { - functions, err := r.getFunctions(ctx, false) - if err != nil { - return nil, err - } - return functions, nil -} - func (r *cachedRepository) getRefreshError() error { r.mutex.Lock() defer r.mutex.Unlock() @@ -179,35 +167,6 @@ func (r *cachedRepository) getCachedPackages(ctx context.Context, forceRefresh b return packages, packageRevisions, err } -func (r *cachedRepository) getFunctions(ctx context.Context, force bool) ([]repository.Function, error) { - var functions []repository.Function - - if !force { - r.mutex.Lock() - functions = r.cachedFunctions - r.mutex.Unlock() - } - - if functions == nil { - fr, ok := (r.repo).(repository.FunctionRepository) - if !ok { - return []repository.Function{}, nil - } - - if f, err := fr.ListFunctions(ctx); err != nil { - return nil, err - } else { - functions = f - } - - r.mutex.Lock() - r.cachedFunctions = functions - r.mutex.Unlock() - } - - return functions, nil -} - func (r *cachedRepository) CreatePackageRevision(ctx context.Context, obj *v1alpha1.PackageRevision) (repository.PackageDraft, error) { created, err := r.repo.CreatePackageRevision(ctx, obj) if err != nil { @@ -382,9 +341,6 @@ func (r *cachedRepository) pollOnce(ctx context.Context) { //if _, err := r.getPackages(ctx, repository.ListPackageRevisionFilter{}, true); err != nil { // klog.Warningf("error polling repo packages %s: %v", r.id, err) //} - if _, err := r.getFunctions(ctx, true); err != nil { - klog.Warningf("error polling repo functions %s: %v", r.id, err) - } } func (r *cachedRepository) flush() { diff --git a/pkg/cli/commands/repo/reg/command.go b/pkg/cli/commands/repo/reg/command.go index 7121e205..80337b76 100644 --- a/pkg/cli/commands/repo/reg/command.go +++ b/pkg/cli/commands/repo/reg/command.go @@ -165,7 +165,6 @@ func (r *runner) runE(_ *cobra.Command, args []string) error { Spec: configapi.RepositorySpec{ Description: r.description, Type: rt, - Content: configapi.RepositoryContentPackage, Deployment: r.deployment, Git: git, Oci: oci, diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index ef42d6c7..8ff622a5 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -62,7 +62,6 @@ type CaDEngine interface { ObjectCache() WatcherManager UpdatePackageResources(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, old, new *api.PackageRevisionResources) (*PackageRevision, *api.RenderStatus, error) - ListFunctions(ctx context.Context, repositoryObj *configapi.Repository) ([]*Function, error) ListPackageRevisions(ctx context.Context, repositorySpec *configapi.Repository, filter repository.ListPackageRevisionFilter) ([]*PackageRevision, error) CreatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, obj *api.PackageRevision, parent *PackageRevision) (*PackageRevision, error) @@ -137,18 +136,6 @@ func (p *PackageRevision) GetResources(ctx context.Context) (*api.PackageRevisio 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) { engine := &cadEngine{} for _, opt := range opts { @@ -1086,30 +1073,6 @@ func applyResourceMutations(ctx context.Context, draft repository.PackageDraft, return applied, renderStatus, nil } -func (cad *cadEngine) ListFunctions(ctx context.Context, repositoryObj *configapi.Repository) ([]*Function, error) { - ctx, span := tracer.Start(ctx, "cadEngine::ListFunctions", trace.WithAttributes()) - defer span.End() - - repo, err := cad.cache.OpenRepository(ctx, repositoryObj) - if err != nil { - return nil, err - } - - fns, err := repo.ListFunctions(ctx) - if err != nil { - return nil, err - } - - 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 diff --git a/pkg/oci/function.go b/pkg/oci/function.go deleted file mode 100644 index 1b6be291..00000000 --- a/pkg/oci/function.go +++ /dev/null @@ -1,174 +0,0 @@ -// Copyright 2022 The kpt and Nephio Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package oci - -import ( - "fmt" - "strings" - "time" - - "github.com/google/go-containerregistry/pkg/name" - "github.com/nephio-project/porch/api/porch/v1alpha1" - configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" - "github.com/nephio-project/porch/pkg/repository" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -const ( - ociImagePrefix = "dev.kpt.fn.meta." - FunctionTypesKey = ociImagePrefix + "types" - DescriptionKey = ociImagePrefix + "description" - DocumentationURLKey = ociImagePrefix + "documentationurl" - keywordsKey = ociImagePrefix + "keywords" - - fnConfigMetaPrefix = ociImagePrefix + "fnconfig." - // experimental: this field is very likely to be changed in the future. - ConfigMapFnKey = fnConfigMetaPrefix + "configmap.requiredfields" -) - -func AnnotationToSlice(annotation string) []string { - vals := strings.Split(annotation, ",") - var result []string - for _, val := range vals { - result = append(result, strings.TrimSpace(val)) - } - return result -} - -type functionMeta struct { - FunctionTypes []string - Description string - DocumentationUrl string - Keywords []string - // experimental: this field is very likely to be changed in the future. - FunctionConfigs []functionConfig -} - -// experimental: this struct is very likely to be changed in the future. -type functionConfig struct { - metav1.TypeMeta - RequiredFields []string -} - -type ociFunction struct { - ref name.Reference - tag name.Reference - name string - version string - created time.Time - meta *functionMeta - parent *ociRepository -} - -var _ repository.Function = &ociFunction{} - -// LINT.IfChange(Name) -func (f *ociFunction) Name() string { - return fmt.Sprintf("%s:%s:%s", f.parent.name, f.name, f.version) - // LINT.ThenChange(internal/fnruntime/container.go AddDefaultImagePathPrefix) -} - -func (f *ociFunction) GetFunction() (*v1alpha1.Function, error) { - var functionTypes []v1alpha1.FunctionType - for _, fnType := range f.meta.FunctionTypes { - switch { - case fnType == string(v1alpha1.FunctionTypeMutator): - functionTypes = append(functionTypes, v1alpha1.FunctionTypeMutator) - case fnType == string(v1alpha1.FunctionTypeValidator): - functionTypes = append(functionTypes, v1alpha1.FunctionTypeValidator) - default: - // unrecognized custom FunctionType - } - } - var fnConfigs []v1alpha1.FunctionConfig - for _, metaFnConfig := range f.meta.FunctionConfigs { - fnConfigs = append(fnConfigs, v1alpha1.FunctionConfig{ - TypeMeta: metaFnConfig.TypeMeta, - RequiredFields: metaFnConfig.RequiredFields, - }) - } - return &v1alpha1.Function{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: f.Name(), - Namespace: f.parent.namespace, - UID: "", - CreationTimestamp: metav1.Time{ - Time: f.created, - }, - }, - Spec: v1alpha1.FunctionSpec{ - Image: f.tag.Name(), - RepositoryRef: v1alpha1.RepositoryRef{ - Name: f.parent.name, - }, - FunctionTypes: functionTypes, - Description: f.meta.Description, - DocumentationUrl: f.meta.DocumentationUrl, - Keywords: f.meta.Keywords, - FunctionConfigs: fnConfigs, - }, - Status: v1alpha1.FunctionStatus{}, - }, nil -} - -func (f *ociFunction) GetCRD() (*configapi.Function, error) { - var functionTypes []configapi.FunctionType - for _, fnType := range f.meta.FunctionTypes { - switch { - case fnType == string(v1alpha1.FunctionTypeMutator): - functionTypes = append(functionTypes, configapi.FunctionTypeMutator) - case fnType == string(v1alpha1.FunctionTypeValidator): - functionTypes = append(functionTypes, configapi.FunctionTypeValidator) - default: - // unrecognized custom FunctionType - } - } - var fnConfigs []configapi.FunctionConfig - for _, metaFnConfig := range f.meta.FunctionConfigs { - fnConfigs = append(fnConfigs, configapi.FunctionConfig{ - TypeMeta: metaFnConfig.TypeMeta, - RequiredFields: metaFnConfig.RequiredFields, - }) - } - - name := fmt.Sprintf("%s.%s.%s", f.parent.name, f.name, f.version) - - return &configapi.Function{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: f.parent.namespace, - }, - Spec: configapi.FunctionSpec{ - Image: f.tag.Name(), - RepositoryRef: configapi.RepositoryRef{ - Name: f.parent.name, - }, - FunctionTypes: functionTypes, - Description: f.meta.Description, - DocumentationUrl: f.meta.DocumentationUrl, - Keywords: f.meta.Keywords, - FunctionConfigs: fnConfigs, - }, - }, nil -} - -// RepositoryStr is the repository part of the resource name, -// typically slash-separated. Last segment is the base function name. -func parseFunctionName(repositoryStr string) string { - slash := strings.LastIndex(repositoryStr, "/") - return repositoryStr[slash+1:] -} diff --git a/pkg/oci/oci.go b/pkg/oci/oci.go index de629486..47c0dd68 100644 --- a/pkg/oci/oci.go +++ b/pkg/oci/oci.go @@ -25,11 +25,8 @@ import ( "time" "github.com/GoogleContainerTools/kpt/pkg/oci" - "github.com/google/go-containerregistry/pkg/gcrane" "github.com/google/go-containerregistry/pkg/name" - v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/google" - "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/nephio-project/porch/api/porch/v1alpha1" configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" "github.com/nephio-project/porch/internal/kpt/pkg" @@ -40,11 +37,10 @@ import ( "k8s.io/klog/v2" ) -func OpenRepository(name string, namespace string, content configapi.RepositoryContent, spec *configapi.OciRepository, deployment bool, storage *oci.Storage) (repository.Repository, error) { +func OpenRepository(name string, namespace string, spec *configapi.OciRepository, deployment bool, storage *oci.Storage) (repository.Repository, error) { return &ociRepository{ name: name, namespace: namespace, - content: content, spec: *spec.DeepCopy(), deployment: deployment, storage: storage, @@ -55,7 +51,6 @@ func OpenRepository(name string, namespace string, content configapi.RepositoryC type ociRepository struct { name string namespace string - content configapi.RepositoryContent spec configapi.OciRepository deployment bool @@ -63,7 +58,6 @@ type ociRepository struct { } var _ repository.Repository = &ociRepository{} -var _ repository.FunctionRepository = &ociRepository{} func (r *ociRepository) Close() error { return nil @@ -74,10 +68,6 @@ func (r *ociRepository) Version(ctx context.Context) (string, error) { ctx, span := tracer.Start(ctx, "ociRepository::Version") defer span.End() - if r.content != configapi.RepositoryContentPackage { - return "", nil - } - ociRepo, err := name.NewRepository(r.spec.Registry) if err != nil { return "", err @@ -123,10 +113,6 @@ func (r *ociRepository) Version(ctx context.Context) (string, error) { } func (r *ociRepository) ListPackageRevisions(ctx context.Context, filter repository.ListPackageRevisionFilter) ([]repository.PackageRevision, error) { - if r.content != configapi.RepositoryContentPackage { - return []repository.PackageRevision{}, nil - } - ctx, span := tracer.Start(ctx, "ociRepository::ListPackageRevisions") defer span.End() @@ -222,9 +208,6 @@ func (r *ociRepository) ListPackages(ctx context.Context, filter repository.List func (r *ociRepository) buildPackageRevision(ctx context.Context, name oci.ImageDigestName, packageName string, workspace v1alpha1.WorkspaceName, revision string, created time.Time) (repository.PackageRevision, error) { - if r.content != configapi.RepositoryContentPackage { - return nil, fmt.Errorf("repository is not a package repo, type is %v", r.content) - } ctx, span := tracer.Start(ctx, "ociRepository::buildPackageRevision") defer span.End() @@ -261,132 +244,6 @@ func (r *ociRepository) buildPackageRevision(ctx context.Context, name oci.Image return p, nil } -func GetFunctionMeta(reference string, ctx context.Context) (*functionMeta, error) { - ref, err := name.ParseReference(reference) - if err != nil { - return nil, fmt.Errorf("parse image reference %v: %v", reference, err) - } - image, err := remote.Image(ref, remote.WithAuthFromKeychain(gcrane.Keychain), remote.WithContext(ctx)) - if err != nil { - return nil, fmt.Errorf("pull remote image %v: %v", reference, err) - } - manifest, err := image.Manifest() - if err != nil { - return nil, fmt.Errorf("get manifest from image %v: %v", reference, err) - } - return &functionMeta{ - FunctionTypes: GetSliceFromAnnotation(FunctionTypesKey, manifest), - Description: GetSingleFromAnnotation(DescriptionKey, manifest), - DocumentationUrl: GetSingleFromAnnotation(DocumentationURLKey, manifest), - Keywords: GetSliceFromAnnotation(keywordsKey, manifest), - FunctionConfigs: GetDefaultFunctionConfig(manifest), - }, nil -} - -func GetDefaultFunctionConfig(manifest *v1.Manifest) []functionConfig { - val, ok := manifest.Annotations[ConfigMapFnKey] - if !ok { - return nil - } - return []functionConfig{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "ConfigMap", - APIVersion: "v1", - }, - RequiredFields: AnnotationToSlice(val), - }, - } -} - -func GetSliceFromAnnotation(key string, manifest *v1.Manifest) []string { - slice, ok := manifest.Annotations[key] - if !ok { - return nil - } - return AnnotationToSlice(slice) -} - -func GetSingleFromAnnotation(key string, manifest *v1.Manifest) string { - if val, ok := manifest.Annotations[key]; ok { - return val - } - return fmt.Sprintf("annotation %v unset", key) -} - -func (r *ociRepository) ListFunctions(ctx context.Context) ([]repository.Function, error) { - // Repository whose content type is not Function contains no Function resources. - if r.content != configapi.RepositoryContentFunction { - klog.Infof("Repository %q doesn't contain functions; contains %s", r.name, r.content) - return []repository.Function{}, nil - } - - ctx, span := tracer.Start(ctx, "ociRepository::ListFunctions") - defer span.End() - - ociRepo, err := name.NewRepository(r.spec.Registry) - if err != nil { - return nil, err - } - - options := r.storage.CreateOptions(ctx) - - result := []repository.Function{} - - err = google.Walk(ociRepo, func(repo name.Repository, tags *google.Tags, err error) error { - if err != nil { - klog.Warningf(" Walk %s encountered error: %v", repo, err) - return err - } - - if tags == nil { - return nil - } - - if cl := len(tags.Children); cl > 0 { - // Expect no manifests or tags - if ml, tl := len(tags.Manifests), len(tags.Tags); ml != 0 || tl != 0 { - return fmt.Errorf("OCI repository with children (%d) as well as Manifests (%d) or Tags (%d)", cl, ml, tl) - } - return nil - } - - functionName := parseFunctionName(repo.RepositoryStr()) - - for digest, manifest := range tags.Manifests { - // Only consider tagged images. - for _, tag := range manifest.Tags { - - created := manifest.Created - if created.IsZero() { - created = manifest.Uploaded - } - meta, err := GetFunctionMeta(repo.Digest(digest).Name(), ctx) - if err != nil { - klog.Warningf(" pull function %v error: %v", functionName, err) - continue - } - result = append(result, &ociFunction{ - ref: repo.Digest(digest), - tag: repo.Tag(tag), - name: functionName, - version: tag, - meta: meta, - created: created, - parent: r, - }) - } - } - - return nil - }, options...) - if err != nil { - return nil, err - } - - return result, nil -} - type ociPackageRevision struct { digestName oci.ImageDigestName packageName string diff --git a/pkg/registry/porch/functions.go b/pkg/registry/porch/functions.go deleted file mode 100644 index ab4d53fa..00000000 --- a/pkg/registry/porch/functions.go +++ /dev/null @@ -1,155 +0,0 @@ -// Copyright 2022 The kpt and Nephio Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package porch - -import ( - "context" - "fmt" - "strings" - - "github.com/nephio-project/porch/api/porch/v1alpha1" - configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" - "github.com/nephio-project/porch/pkg/engine" - apierrors "k8s.io/apimachinery/pkg/api/errors" - metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apiserver/pkg/endpoints/request" - "k8s.io/apiserver/pkg/registry/rest" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -type functions struct { - rest.TableConvertor - - cad engine.CaDEngine - coreClient client.Client -} - -var _ rest.Storage = &functions{} -var _ rest.Scoper = &functions{} -var _ rest.Lister = &functions{} -var _ rest.Getter = &functions{} -var _ rest.SingularNameProvider = &functions{} - - -// GetSingularName implements the SingularNameProvider interface -func (r *functions) GetSingularName() (string) { - return "function" -} - - -func (f *functions) New() runtime.Object { - return &v1alpha1.Function{} -} - -func (f *functions) Destroy() {} - -func (f *functions) NamespaceScoped() bool { - return true -} - -func (f *functions) NewList() runtime.Object { - return &v1alpha1.FunctionList{} -} - -// List selects resources in the storage which match to the selector. 'options' can be nil. -func (f *functions) List(ctx context.Context, options *metainternalversion.ListOptions) (runtime.Object, error) { - var opts []client.ListOption - if ns, ok := request.NamespaceFrom(ctx); ok { - opts = append(opts, client.InNamespace(ns)) - } - - var repositories configapi.RepositoryList - if err := f.coreClient.List(ctx, &repositories, opts...); err != nil { - return nil, fmt.Errorf("failed to list registered repositories: %w", err) - } - - result := &v1alpha1.FunctionList{} - - for i := range repositories.Items { - repo := &repositories.Items[i] - fns, err := f.cad.ListFunctions(ctx, repo) - if err != nil { - return nil, fmt.Errorf("failed to list repository %s functions: %w", repositories.Items[i].Name, err) - } - for _, f := range fns { - api, err := f.GetFunction() - if err != nil { - return nil, fmt.Errorf("failed to get function details %s: %w", f.Name(), err) - } - result.Items = append(result.Items, *api) - } - } - - return result, nil -} - -func (f *functions) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { - var repositoryKey client.ObjectKey - if ns, ok := request.NamespaceFrom(ctx); !ok { - return nil, apierrors.NewBadRequest("namespace must be specified") - } else { - repositoryKey.Namespace = ns - } - - if fn, err := parseFunctionName(name); err != nil { - return nil, apierrors.NewNotFound(schema.GroupResource(v1alpha1.FunctionGVR.GroupResource()), name) - } else { - repositoryKey.Name = fn.repository - } - - var repository configapi.Repository - if err := f.coreClient.Get(ctx, repositoryKey, &repository); err != nil { - if apierrors.IsNotFound(err) { - return nil, apierrors.NewNotFound(schema.GroupResource(v1alpha1.FunctionGVR.GroupResource()), name) - } - return nil, fmt.Errorf("error getting repository %s: %w", repositoryKey, err) - } - - // TODO: implement get to avoid listing - fns, err := f.cad.ListFunctions(ctx, &repository) - if err != nil { - return nil, fmt.Errorf("failed to list repository %s functions: %w", repository.Name, err) - } - - for _, f := range fns { - if f.Name() == name { - return f.GetFunction() - } - } - - return nil, apierrors.NewNotFound(schema.GroupResource(v1alpha1.FunctionGVR.GroupResource()), name) -} - -type functionName struct { - repository, name, version string -} - -func parseFunctionName(name string) (functionName, error) { - var result functionName - - parts := strings.SplitN(name, ":", 3) - if len(parts) != 3 { - return result, fmt.Errorf("invalid name %q; expect name in the format ::", name) - } - - result.repository = parts[0] - result.name = parts[1] - result.version = parts[2] - - return result, nil -} diff --git a/pkg/registry/porch/storage.go b/pkg/registry/porch/storage.go index 3d3d5e36..874aa05a 100644 --- a/pkg/registry/porch/storage.go +++ b/pkg/registry/porch/storage.go @@ -73,12 +73,6 @@ func NewRESTStorage(scheme *runtime.Scheme, codecs serializer.CodecFactory, cad }, } - functions := &functions{ - TableConvertor: rest.NewDefaultTableConvertor(porch.Resource("functions")), - cad: cad, - coreClient: coreClient, - } - group := genericapiserver.NewDefaultAPIGroupInfo(porch.GroupName, scheme, metav1.ParameterCodec, codecs) group.VersionedResourcesStorageMap = map[string]map[string]rest.Storage{ @@ -87,7 +81,6 @@ func NewRESTStorage(scheme *runtime.Scheme, codecs serializer.CodecFactory, cad "packagerevisions": packageRevisions, "packagerevisions/approval": packageRevisionsApproval, "packagerevisionresources": packageRevisionResources, - "functions": functions, }, } diff --git a/pkg/repository/repository.go b/pkg/repository/repository.go index d36dd783..68837045 100644 --- a/pkg/repository/repository.go +++ b/pkg/repository/repository.go @@ -20,7 +20,6 @@ import ( "github.com/go-git/go-git/v5/plumbing/transport" "github.com/nephio-project/porch/api/porch/v1alpha1" - configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" kptfile "github.com/nephio-project/porch/pkg/kpt/api/kptfile/v1" "k8s.io/apimachinery/pkg/types" ) @@ -121,13 +120,6 @@ type PackageDraft interface { Close(ctx context.Context) (PackageRevision, error) } -// Function is an abstract function. -type Function interface { - Name() string - GetFunction() (*v1alpha1.Function, error) - GetCRD() (*configapi.Function, error) -} - // ListPackageRevisionFilter is a predicate for filtering PackageRevision objects; // only matching PackageRevision objects will be returned. type ListPackageRevisionFilter struct { @@ -221,11 +213,6 @@ type Repository interface { Close() error } -type FunctionRepository interface { - // TODO: Should repository understand functions, or just packages (and function is just a package in an OCI repo?) - ListFunctions(ctx context.Context) ([]Function, error) -} - // The definitions below would be more appropriately located in a package usable by any Porch component. // They are located in repository package because repository is one such package though thematically // they rather belong to a package of their own. diff --git a/test/e2e/cli/testdata/repo-register/config.yaml b/test/e2e/cli/testdata/repo-register/config.yaml index 95f1eea2..8fda8945 100644 --- a/test/e2e/cli/testdata/repo-register/config.yaml +++ b/test/e2e/cli/testdata/repo-register/config.yaml @@ -21,18 +21,18 @@ commands: - repo - get - --namespace=repo-register - - --output=custom-columns=NAME:.metadata.name,CONTENT:.spec.content,DESC:.spec.description + - --output=custom-columns=NAME:.metadata.name,DESC:.spec.description stdout: | - NAME CONTENT DESC - test-blueprints Package Test Blueprints + NAME DESC + test-blueprints Test Blueprints - args: - porchctl - repo - get - --namespace=repo-register stdout: | - NAME TYPE CONTENT DEPLOYMENT READY ADDRESS - test-blueprints git Package True https://github.com/platkrm/test-blueprints.git + NAME TYPE DEPLOYMENT READY ADDRESS + test-blueprints git True https://github.com/platkrm/test-blueprints.git - args: - porchctl - repo diff --git a/test/e2e/cli/testdata/rpkg-init-deploy/config.yaml b/test/e2e/cli/testdata/rpkg-init-deploy/config.yaml index 33080679..a5fbc814 100644 --- a/test/e2e/cli/testdata/rpkg-init-deploy/config.yaml +++ b/test/e2e/cli/testdata/rpkg-init-deploy/config.yaml @@ -87,5 +87,5 @@ commands: - get - --namespace=rpkg-init-deploy stdout: | - NAME TYPE CONTENT DEPLOYMENT READY ADDRESS - git git Package true True http://git-server.test-git-namespace.svc.cluster.local:8080/rpkg-init-deploy + NAME TYPE DEPLOYMENT READY ADDRESS + git git true True http://git-server.test-git-namespace.svc.cluster.local:8080/rpkg-init-deploy diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index e5e81c82..8cab7fc6 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -16,7 +16,6 @@ package e2e import ( "context" - "errors" "fmt" "os" "path/filepath" @@ -33,12 +32,10 @@ import ( "github.com/nephio-project/porch/pkg/repository" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/wait" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/kustomize/kyaml/yaml" @@ -881,72 +878,6 @@ func (t *PorchSuite) TestConcurrentResourceUpdates(ctx context.Context) { assert.True(t, conflictFailurePresent, "expected one request to fail with a conflict, but did not happen - results: %v", results) } -func (t *PorchSuite) TestFunctionRepository(ctx context.Context) { - repo := &configapi.Repository{ - ObjectMeta: metav1.ObjectMeta{ - Name: "function-repository", - Namespace: t.Namespace, - }, - Spec: configapi.RepositorySpec{ - Description: "Test Function Repository", - Type: configapi.RepositoryTypeOCI, - Content: configapi.RepositoryContentFunction, - Oci: &configapi.OciRepository{ - Registry: "gcr.io/kpt-fn", - }, - }, - } - t.CreateF(ctx, repo) - - t.Cleanup(func() { - t.DeleteL(ctx, &configapi.Repository{ - ObjectMeta: metav1.ObjectMeta{ - Name: repo.Name, - Namespace: repo.Namespace, - }, - }) - }) - - // Make sure the repository is ready before we test to (hopefully) - // avoid flakiness. - t.WaitUntilRepositoryReady(ctx, repo.Name, repo.Namespace) - - t.Log("Waiting for 1 minute to avoid most of the initial transient period") - <-time.NewTimer(1 * time.Minute).C - - t.Log("Waiting for the gcr.io/kpt-fn repository to be cached in porch") - timeout := 5 * time.Minute - list := porchapi.FunctionList{} - err := wait.PollUntilContextTimeout(ctx, 5*time.Second, timeout, false, func(ctx context.Context) (bool, error) { - err := t.Client.List(ctx, &list, client.InNamespace(t.Namespace)) - if err != nil { - if apierrors.IsTimeout(err) || - errors.Is(err, os.ErrDeadlineExceeded) || - errors.Is(err, context.DeadlineExceeded) { - // in case of a timeout, we simply retry - t.Logf("timeout -> retry: %v", err) - return false, nil - } else { - // unfortunately, transient errors can happen during normal operation, so we retry instead of failing - t.Logf("Warning: error listing functions in gcr.io/kpt-fn repository: %v", err) - return false, nil - } - } - if len(list.Items) == 0 { - // unfortunately, sometimes an empty list is returned without any errors during the transient period, so we retry instead of failing - t.Log("Warning: an empty list of functions was returned (temporarily?)") - return false, nil - } - return true, nil - }) - if err != nil { - t.Errorf("Error listing functions in gcr.io/kpt-fn repository with timeout of %v: %v", timeout, err) - } - if got := len(list.Items); got == 0 { - t.Errorf("Found no functions in gcr.io/kpt-fn repository; expected at least one") - } -} - func (t *PorchSuite) NewClientWithTimeout(timeout time.Duration) client.Client { cfg := *t.Kubeconfig cfg.Timeout = timeout @@ -1815,7 +1746,6 @@ func (t *PorchSuite) TestRegisterRepository(ctx context.Context) { repository = "register" ) t.RegisterMainGitRepositoryF(ctx, repository, - withContent(configapi.RepositoryContentPackage), withType(configapi.RepositoryTypeGit), WithDeployment()) @@ -1825,9 +1755,6 @@ func (t *PorchSuite) TestRegisterRepository(ctx context.Context) { Name: repository, }, &repo) - if got, want := repo.Spec.Content, configapi.RepositoryContentPackage; got != want { - t.Errorf("Repo Content: got %q, want %q", got, want) - } if got, want := repo.Spec.Type, configapi.RepositoryTypeGit; got != want { t.Errorf("Repo Type: got %q, want %q", got, want) } @@ -2296,7 +2223,6 @@ func (t *PorchSuite) TestRepositoryError(ctx context.Context) { Spec: configapi.RepositorySpec{ Description: "Repository With Error", Type: configapi.RepositoryTypeGit, - Content: configapi.RepositoryContentPackage, Git: &configapi.GitRepository{ // Use `invalid` domain: https://www.rfc-editor.org/rfc/rfc6761#section-6.4 Repo: "https://repo.invalid/repository.git", diff --git a/test/e2e/suite_utils.go b/test/e2e/suite_utils.go index ccf0c170..d1c5c53c 100644 --- a/test/e2e/suite_utils.go +++ b/test/e2e/suite_utils.go @@ -197,7 +197,6 @@ func (t *TestSuite) registerGitRepositoryFromConfigF(ctx context.Context, name s Spec: configapi.RepositorySpec{ Description: "Porch Test Repository Description", Type: configapi.RepositoryTypeGit, - Content: configapi.RepositoryContentPackage, Git: &configapi.GitRepository{ Repo: config.Repo, Branch: config.Branch, @@ -243,12 +242,6 @@ func withType(t configapi.RepositoryType) RepositoryOption { } } -func withContent(content configapi.RepositoryContent) RepositoryOption { - return func(r *configapi.Repository) { - r.Spec.Content = content - } -} - func InNamespace(ns string) RepositoryOption { return func(repo *configapi.Repository) { repo.Namespace = ns From 1791a3843ebc973caff0da762d4953ba3f572081 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Sat, 5 Oct 2024 10:57:32 +0100 Subject: [PATCH 2/3] Fix failing Repo tests --- pkg/cli/commands/repo/reg/testdata/auth-repository.yaml | 1 - pkg/cli/commands/repo/reg/testdata/full-repository.yaml | 1 - pkg/cli/commands/repo/reg/testdata/simple-repository.yaml | 1 - 3 files changed, 3 deletions(-) diff --git a/pkg/cli/commands/repo/reg/testdata/auth-repository.yaml b/pkg/cli/commands/repo/reg/testdata/auth-repository.yaml index a7deb45c..4797ffe9 100644 --- a/pkg/cli/commands/repo/reg/testdata/auth-repository.yaml +++ b/pkg/cli/commands/repo/reg/testdata/auth-repository.yaml @@ -5,7 +5,6 @@ metadata: name: test-blueprints namespace: default spec: - content: Package git: branch: main directory: / diff --git a/pkg/cli/commands/repo/reg/testdata/full-repository.yaml b/pkg/cli/commands/repo/reg/testdata/full-repository.yaml index a3650ae6..86096d0c 100644 --- a/pkg/cli/commands/repo/reg/testdata/full-repository.yaml +++ b/pkg/cli/commands/repo/reg/testdata/full-repository.yaml @@ -5,7 +5,6 @@ metadata: name: repository-resource-name namespace: repository-namespace spec: - content: Package deployment: true description: '"Test Repository Description"' git: diff --git a/pkg/cli/commands/repo/reg/testdata/simple-repository.yaml b/pkg/cli/commands/repo/reg/testdata/simple-repository.yaml index 9739eb97..b870d77c 100644 --- a/pkg/cli/commands/repo/reg/testdata/simple-repository.yaml +++ b/pkg/cli/commands/repo/reg/testdata/simple-repository.yaml @@ -5,7 +5,6 @@ metadata: name: test-blueprints namespace: default spec: - content: Package git: branch: main directory: / From 12f7597b0581bb1e2550eb0d71be4ee42ed4aa7e Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Tue, 5 Nov 2024 12:44:19 +0000 Subject: [PATCH 3/3] Fix rebase again --- .../v1alpha1/config.porch.kpt.dev_repositories.yaml | 13 +++++++++++++ api/porchconfig/v1alpha1/types.go | 13 +++++++++++++ api/porchconfig/v1alpha1/zz_generated.deepcopy.go | 5 +++++ pkg/cache/memory/cache.go | 9 +-------- test/e2e/cli/testdata/repo-register/config.yaml | 4 ++-- test/e2e/cli/testdata/rpkg-init-deploy/config.yaml | 4 ++-- 6 files changed, 36 insertions(+), 12 deletions(-) diff --git a/api/porchconfig/v1alpha1/config.porch.kpt.dev_repositories.yaml b/api/porchconfig/v1alpha1/config.porch.kpt.dev_repositories.yaml index cc6905aa..d706a6f7 100644 --- a/api/porchconfig/v1alpha1/config.porch.kpt.dev_repositories.yaml +++ b/api/porchconfig/v1alpha1/config.porch.kpt.dev_repositories.yaml @@ -18,6 +18,9 @@ spec: - jsonPath: .spec.type name: Type type: string + - jsonPath: .spec.content + name: Content + type: string - jsonPath: .spec.deployment name: Deployment type: boolean @@ -56,6 +59,16 @@ spec: Notes: - deployment repository - in KRM API ConfigSync would be configured directly? (or via this API) properties: + content: + default: Package + description: |- + The Content field is deprecated, please do not specify it in new manifests. + For partial backward compatibility it is still recognized, but its only valid value is "Package", and if not specified its default value is also "Package". + type: string + x-kubernetes-validations: + - message: The 'content' field is deprecated, its only valid value + is 'Package' + rule: self == '' || self == 'Package' deployment: description: The repository is a deployment repository; final packages in this repository are deployment ready. diff --git a/api/porchconfig/v1alpha1/types.go b/api/porchconfig/v1alpha1/types.go index 9183465b..5c7239f1 100644 --- a/api/porchconfig/v1alpha1/types.go +++ b/api/porchconfig/v1alpha1/types.go @@ -22,6 +22,7 @@ import ( //+kubebuilder:subresource:status //+kubebuilder:resource:path=repositories,singular=repository //+kubebuilder:printcolumn:name="Type",type=string,JSONPath=`.spec.type` +//+kubebuilder:printcolumn:name="Content",type=string,JSONPath=`.spec.content` //+kubebuilder:printcolumn:name="Deployment",type=boolean,JSONPath=`.spec.deployment` //+kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=='Ready')].status` //+kubebuilder:printcolumn:name="Address",type=string,JSONPath=`.spec['git','oci']['repo','registry']` @@ -42,6 +43,12 @@ const ( RepositoryTypeOCI RepositoryType = "oci" ) +type RepositoryContent string + +const ( + RepositoryContentPackage RepositoryContent = "Package" +) + // RepositorySpec defines the desired state of Repository // // Notes: @@ -53,6 +60,12 @@ type RepositorySpec struct { Deployment bool `json:"deployment,omitempty"` // Type of the repository (i.e. git, OCI) Type RepositoryType `json:"type,omitempty"` + // The Content field is deprecated, please do not specify it in new manifests. + // For partial backward compatibility it is still recognized, but its only valid value is "Package", and if not specified its default value is also "Package". + // +kubebuilder:validation:XValidation:message="The 'content' field is deprecated, its only valid value is 'Package'",rule="self == '' || self == 'Package'" + // +kubebuilder:default="Package" + Content *RepositoryContent `json:"content,omitempty"` + // Git repository details. Required if `type` is `git`. Ignored if `type` is not `git`. Git *GitRepository `json:"git,omitempty"` // OCI repository details. Required if `type` is `oci`. Ignored if `type` is not `oci`. diff --git a/api/porchconfig/v1alpha1/zz_generated.deepcopy.go b/api/porchconfig/v1alpha1/zz_generated.deepcopy.go index d8d641b0..b637beb4 100644 --- a/api/porchconfig/v1alpha1/zz_generated.deepcopy.go +++ b/api/porchconfig/v1alpha1/zz_generated.deepcopy.go @@ -154,6 +154,11 @@ func (in *RepositoryRef) DeepCopy() *RepositoryRef { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RepositorySpec) DeepCopyInto(out *RepositorySpec) { *out = *in + if in.Content != nil { + in, out := &in.Content, &out.Content + *out = new(RepositoryContent) + **out = **in + } if in.Git != nil { in, out := &in.Git, &out.Git *out = new(GitRepository) diff --git a/pkg/cache/memory/cache.go b/pkg/cache/memory/cache.go index e1c8caeb..1961c41d 100644 --- a/pkg/cache/memory/cache.go +++ b/pkg/cache/memory/cache.go @@ -127,7 +127,7 @@ func (c *Cache) OpenRepository(ctx context.Context, repositorySpec *configapi.Re return nil, err } - r, err := oci.OpenRepository(repositorySpec.Name, repositorySpec.Namespace, repositorySpec.Spec.Content, ociSpec, repositorySpec.Spec.Deployment, storage) + r, err := oci.OpenRepository(repositorySpec.Name, repositorySpec.Namespace, ociSpec, repositorySpec.Spec.Deployment, storage) if err != nil { return nil, err } @@ -138,9 +138,6 @@ func (c *Cache) OpenRepository(ctx context.Context, repositorySpec *configapi.Re case configapi.RepositoryTypeGit: gitSpec := repositorySpec.Spec.Git - if !isPackageContent(repositorySpec.Spec.Content) { - return nil, fmt.Errorf("git repository supports Package content only; got %q", string(repositorySpec.Spec.Content)) - } if cachedRepo == nil { var mbs git.MainBranchStrategy if gitSpec.CreateBranch { @@ -174,10 +171,6 @@ func (c *Cache) OpenRepository(ctx context.Context, repositorySpec *configapi.Re } } -func isPackageContent(content configapi.RepositoryContent) bool { - return content == configapi.RepositoryContentPackage -} - func (c *Cache) CloseRepository(repositorySpec *configapi.Repository, allRepos []configapi.Repository) error { key, err := getCacheKey(repositorySpec) if err != nil { diff --git a/test/e2e/cli/testdata/repo-register/config.yaml b/test/e2e/cli/testdata/repo-register/config.yaml index 8fda8945..5e903814 100644 --- a/test/e2e/cli/testdata/repo-register/config.yaml +++ b/test/e2e/cli/testdata/repo-register/config.yaml @@ -31,8 +31,8 @@ commands: - get - --namespace=repo-register stdout: | - NAME TYPE DEPLOYMENT READY ADDRESS - test-blueprints git True https://github.com/platkrm/test-blueprints.git + NAME TYPE CONTENT DEPLOYMENT READY ADDRESS + test-blueprints git Package True https://github.com/platkrm/test-blueprints.git - args: - porchctl - repo diff --git a/test/e2e/cli/testdata/rpkg-init-deploy/config.yaml b/test/e2e/cli/testdata/rpkg-init-deploy/config.yaml index a5fbc814..33080679 100644 --- a/test/e2e/cli/testdata/rpkg-init-deploy/config.yaml +++ b/test/e2e/cli/testdata/rpkg-init-deploy/config.yaml @@ -87,5 +87,5 @@ commands: - get - --namespace=rpkg-init-deploy stdout: | - NAME TYPE DEPLOYMENT READY ADDRESS - git git true True http://git-server.test-git-namespace.svc.cluster.local:8080/rpkg-init-deploy + NAME TYPE CONTENT DEPLOYMENT READY ADDRESS + git git Package true True http://git-server.test-git-namespace.svc.cluster.local:8080/rpkg-init-deploy