diff --git a/Earthfile b/Earthfile index 1eec76e68..bfdd11d50 100644 --- a/Earthfile +++ b/Earthfile @@ -3,7 +3,7 @@ VERSION --try --raw-output 0.8 PROJECT upbound/crossplane -ARG --global GO_VERSION=1.22.3 +ARG --global GO_VERSION=1.22.8 # reviewable checks that a branch is ready for review. Run it before opening a # pull request. It will catch a lot of the things our CI workflow will catch. diff --git a/apis/pkg/v1beta1/image_config_types.go b/apis/pkg/v1beta1/image_config_types.go new file mode 100644 index 000000000..ec3fa49e7 --- /dev/null +++ b/apis/pkg/v1beta1/image_config_types.go @@ -0,0 +1,89 @@ +/* +Copyright 2024 The Crossplane 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 v1beta1 + +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// MatchType is the method used to match the image. +type MatchType string + +const ( + // Prefix is used to match the prefix of the image. + Prefix MatchType = "Prefix" +) + +// +kubebuilder:object:root=true +// +genclient +// +genclient:nonNamespaced + +// The ImageConfig resource is used to configure settings for package images. +// +// +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=".metadata.creationTimestamp" +// +kubebuilder:resource:scope=Cluster,categories={crossplane} +type ImageConfig struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec ImageConfigSpec `json:"spec,omitempty"` +} + +// +kubebuilder:object:root=true + +// ImageConfigList contains a list of ImageConfig. +type ImageConfigList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []ImageConfig `json:"items"` +} + +// ImageMatch defines a rule for matching image. +type ImageMatch struct { + // Type is the type of match. + // +optional + // +kubebuilder:validation:Enum=Prefix + // +kubebuilder:default=Prefix + Type MatchType `json:"type,omitempty"` + // Prefix is the prefix that should be matched. + Prefix string `json:"prefix"` +} + +// RegistryAuthentication contains the authentication information for a registry. +type RegistryAuthentication struct { + // PullSecretRef is a reference to a secret that contains the credentials for + // the registry. + PullSecretRef corev1.LocalObjectReference `json:"pullSecretRef"` +} + +// RegistryConfig contains the configuration for the registry. +type RegistryConfig struct { + // Authentication is the authentication information for the registry. + // +optional + Authentication *RegistryAuthentication `json:"authentication,omitempty"` +} + +// ImageConfigSpec contains the configuration for matching images. +type ImageConfigSpec struct { + // MatchImages is a list of image matching rules that should be satisfied. + // +kubebuilder:validation:XValidation:rule="size(self) > 0",message="matchImages should have at least one element." + MatchImages []ImageMatch `json:"matchImages"` + // Registry is the configuration for the registry. + // +optional + Registry *RegistryConfig `json:"registry,omitempty"` +} diff --git a/apis/pkg/v1beta1/register.go b/apis/pkg/v1beta1/register.go index 9c70db5f9..9d069efb3 100644 --- a/apis/pkg/v1beta1/register.go +++ b/apis/pkg/v1beta1/register.go @@ -72,9 +72,18 @@ var ( DeploymentRuntimeConfigGroupVersionKind = SchemeGroupVersion.WithKind(DeploymentRuntimeConfigKind) ) +// ImageConfig type metadata. +var ( + ImageConfigKind = reflect.TypeOf(ImageConfig{}).Name() + ImageConfigGroupKind = schema.GroupKind{Group: Group, Kind: ImageConfigKind}.String() + ImageConfigKindAPIVersion = ImageConfigKind + "." + SchemeGroupVersion.String() + ImageConfigGroupVersionKind = SchemeGroupVersion.WithKind(ImageConfigKind) +) + func init() { SchemeBuilder.Register(&Lock{}, &LockList{}) SchemeBuilder.Register(&Function{}, &FunctionList{}) SchemeBuilder.Register(&FunctionRevision{}, &FunctionRevisionList{}) SchemeBuilder.Register(&DeploymentRuntimeConfig{}, &DeploymentRuntimeConfigList{}) + SchemeBuilder.Register(&ImageConfig{}, &ImageConfigList{}) } diff --git a/apis/pkg/v1beta1/zz_generated.deepcopy.go b/apis/pkg/v1beta1/zz_generated.deepcopy.go index 90cfe44fb..2c93bc14b 100644 --- a/apis/pkg/v1beta1/zz_generated.deepcopy.go +++ b/apis/pkg/v1beta1/zz_generated.deepcopy.go @@ -371,6 +371,104 @@ func (in *FunctionStatus) DeepCopy() *FunctionStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ImageConfig) DeepCopyInto(out *ImageConfig) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ImageConfig. +func (in *ImageConfig) DeepCopy() *ImageConfig { + if in == nil { + return nil + } + out := new(ImageConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ImageConfig) 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 *ImageConfigList) DeepCopyInto(out *ImageConfigList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ImageConfig, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ImageConfigList. +func (in *ImageConfigList) DeepCopy() *ImageConfigList { + if in == nil { + return nil + } + out := new(ImageConfigList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ImageConfigList) 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 *ImageConfigSpec) DeepCopyInto(out *ImageConfigSpec) { + *out = *in + if in.MatchImages != nil { + in, out := &in.MatchImages, &out.MatchImages + *out = make([]ImageMatch, len(*in)) + copy(*out, *in) + } + if in.Registry != nil { + in, out := &in.Registry, &out.Registry + *out = new(RegistryConfig) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ImageConfigSpec. +func (in *ImageConfigSpec) DeepCopy() *ImageConfigSpec { + if in == nil { + return nil + } + out := new(ImageConfigSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ImageMatch) DeepCopyInto(out *ImageMatch) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ImageMatch. +func (in *ImageMatch) DeepCopy() *ImageMatch { + if in == nil { + return nil + } + out := new(ImageMatch) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Lock) DeepCopyInto(out *Lock) { *out = *in @@ -677,6 +775,42 @@ func (in *PackageStatus) DeepCopy() *PackageStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RegistryAuthentication) DeepCopyInto(out *RegistryAuthentication) { + *out = *in + out.PullSecretRef = in.PullSecretRef +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RegistryAuthentication. +func (in *RegistryAuthentication) DeepCopy() *RegistryAuthentication { + if in == nil { + return nil + } + out := new(RegistryAuthentication) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RegistryConfig) DeepCopyInto(out *RegistryConfig) { + *out = *in + if in.Authentication != nil { + in, out := &in.Authentication, &out.Authentication + *out = new(RegistryAuthentication) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RegistryConfig. +func (in *RegistryConfig) DeepCopy() *RegistryConfig { + if in == nil { + return nil + } + out := new(RegistryConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RuntimeConfigReference) DeepCopyInto(out *RuntimeConfigReference) { *out = *in diff --git a/cluster/crds/pkg.crossplane.io_imageconfigs.yaml b/cluster/crds/pkg.crossplane.io_imageconfigs.yaml new file mode 100644 index 000000000..19774ff64 --- /dev/null +++ b/cluster/crds/pkg.crossplane.io_imageconfigs.yaml @@ -0,0 +1,101 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.14.0 + name: imageconfigs.pkg.crossplane.io +spec: + group: pkg.crossplane.io + names: + categories: + - crossplane + kind: ImageConfig + listKind: ImageConfigList + plural: imageconfigs + singular: imageconfig + scope: Cluster + versions: + - additionalPrinterColumns: + - jsonPath: .metadata.creationTimestamp + name: AGE + type: date + name: v1beta1 + schema: + openAPIV3Schema: + description: The ImageConfig resource is used to configure settings for package + images. + 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: ImageConfigSpec contains the configuration for matching images. + properties: + matchImages: + description: MatchImages is a list of image matching rules that should + be satisfied. + items: + description: ImageMatch defines a rule for matching image. + properties: + prefix: + description: Prefix is the prefix that should be matched. + type: string + type: + default: Prefix + description: Type is the type of match. + enum: + - Prefix + type: string + required: + - prefix + type: object + type: array + x-kubernetes-validations: + - message: matchImages should have at least one element. + rule: size(self) > 0 + registry: + description: Registry is the configuration for the registry. + properties: + authentication: + description: Authentication is the authentication information + for the registry. + properties: + pullSecretRef: + description: |- + PullSecretRef is a reference to a secret that contains the credentials for + the registry. + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + required: + - pullSecretRef + type: object + type: object + required: + - matchImages + type: object + type: object + served: true + storage: true + subresources: {} diff --git a/cmd/crank/render/render.go b/cmd/crank/render/render.go index 9e8accda3..24df0591c 100644 --- a/cmd/crank/render/render.go +++ b/cmd/crank/render/render.go @@ -124,7 +124,7 @@ func NewRuntimeFunctionRunner(ctx context.Context, log logging.Logger, fns []pkg conns[fn.GetName()] = conn } - return &RuntimeFunctionRunner{conns: conns}, nil + return &RuntimeFunctionRunner{contexts: contexts, conns: conns}, nil } // RunFunction runs the named function. diff --git a/cmd/crank/render/runtime_docker.go b/cmd/crank/render/runtime_docker.go index 92cfee7f8..2988807d9 100644 --- a/cmd/crank/render/runtime_docker.go +++ b/cmd/crank/render/runtime_docker.go @@ -22,8 +22,8 @@ import ( "io" "net" - "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" + typesimage "github.com/docker/docker/api/types/image" "github.com/docker/docker/client" "github.com/docker/docker/errdefs" "github.com/docker/go-connections/nat" @@ -246,13 +246,13 @@ func (r *RuntimeDocker) Start(ctx context.Context) (RuntimeContext, error) { } type pullClient interface { - ImagePull(ctx context.Context, ref string, options types.ImagePullOptions) (io.ReadCloser, error) + ImagePull(ctx context.Context, ref string, options typesimage.PullOptions) (io.ReadCloser, error) } // PullImage pulls the supplied image using the supplied client. It blocks until // the image has either finished pulling or hit an error. func PullImage(ctx context.Context, p pullClient, image string) error { - out, err := p.ImagePull(ctx, image, types.ImagePullOptions{}) + out, err := p.ImagePull(ctx, image, typesimage.PullOptions{}) if err != nil { return err } diff --git a/cmd/crank/render/runtime_docker_test.go b/cmd/crank/render/runtime_docker_test.go index 913e5f0d0..6cbf25928 100644 --- a/cmd/crank/render/runtime_docker_test.go +++ b/cmd/crank/render/runtime_docker_test.go @@ -21,7 +21,7 @@ import ( "io" "testing" - "github.com/docker/docker/api/types" + typesimage "github.com/docker/docker/api/types/image" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -32,10 +32,10 @@ import ( ) type mockPullClient struct { - MockPullImage func(_ context.Context, ref string, options types.ImagePullOptions) (io.ReadCloser, error) + MockPullImage func(_ context.Context, ref string, options typesimage.PullOptions) (io.ReadCloser, error) } -func (m *mockPullClient) ImagePull(ctx context.Context, ref string, options types.ImagePullOptions) (io.ReadCloser, error) { +func (m *mockPullClient) ImagePull(ctx context.Context, ref string, options typesimage.PullOptions) (io.ReadCloser, error) { return m.MockPullImage(ctx, ref, options) } diff --git a/go.mod b/go.mod index 92b78138f..4ddae1bab 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,6 @@ module github.com/crossplane/crossplane -go 1.22.0 - -toolchain go1.22.3 +go 1.22.8 require ( dario.cat/mergo v1.0.0 @@ -10,7 +8,7 @@ require ( github.com/Masterminds/semver v1.5.0 github.com/alecthomas/kong v0.9.0 github.com/crossplane/crossplane-runtime v1.17.0 - github.com/docker/docker v25.0.6+incompatible + github.com/docker/docker v27.1.1+incompatible github.com/docker/go-connections v0.5.0 github.com/emicklei/dot v1.6.2 github.com/go-git/go-billy/v5 v5.5.0 @@ -61,7 +59,7 @@ require ( github.com/felixge/httpsnoop v1.0.4 // indirect github.com/go-errors/errors v1.4.2 // indirect github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect - github.com/golang-jwt/jwt/v4 v4.5.0 // indirect + github.com/golang-jwt/jwt/v4 v4.5.1 // indirect github.com/google/btree v1.0.1 // indirect github.com/google/cel-go v0.17.8 // indirect github.com/google/gnostic-models v0.6.8 // indirect @@ -72,6 +70,7 @@ require ( github.com/kevinburke/ssh_config v1.2.0 // indirect github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de // indirect github.com/mitchellh/go-wordwrap v1.0.1 // indirect + github.com/moby/docker-image-spec v1.3.1 // indirect github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect github.com/peterbourgon/diskv v2.0.1+incompatible // indirect diff --git a/go.sum b/go.sum index 36cc8985e..bc1f82983 100644 --- a/go.sum +++ b/go.sum @@ -150,8 +150,8 @@ github.com/docker/cli v24.0.7+incompatible h1:wa/nIwYFW7BVTGa7SWPVyyXU9lgORqUb1x github.com/docker/cli v24.0.7+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= github.com/docker/distribution v2.8.3+incompatible h1:AtKxIZ36LoNK51+Z6RpzLpddBirtxJnzDrHLEKxTAYk= github.com/docker/distribution v2.8.3+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= -github.com/docker/docker v25.0.6+incompatible h1:5cPwbwriIcsua2REJe8HqQV+6WlWc1byg2QSXzBxBGg= -github.com/docker/docker v25.0.6+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= +github.com/docker/docker v27.1.1+incompatible h1:hO/M4MtV36kzKldqnA37IWhebRA+LnqqcqDja6kVaKY= +github.com/docker/docker v27.1.1+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= github.com/docker/docker-credential-helpers v0.7.0/go.mod h1:rETQfLdHNT3foU5kuNkFR1R1V12OJRRO5lzt2D1b5X0= github.com/docker/docker-credential-helpers v0.8.2 h1:bX3YxiGzFP5sOXWc3bTPEXdEaZSeVMrFgOr3T+zrFAo= github.com/docker/docker-credential-helpers v0.8.2/go.mod h1:P3ci7E3lwkZg6XiHdRKft1KckHiO9a2rNtyFbZ/ry9M= @@ -215,8 +215,9 @@ github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= github.com/golang-jwt/jwt/v4 v4.0.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg= github.com/golang-jwt/jwt/v4 v4.2.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg= -github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg= github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= +github.com/golang-jwt/jwt/v4 v4.5.1 h1:JdqV9zKUdtaa9gdPlywC3aeoEsR681PlKC+4F5gQgeo= +github.com/golang-jwt/jwt/v4 v4.5.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk= github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= @@ -326,6 +327,8 @@ github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/go-wordwrap v1.0.1 h1:TLuKupo69TCn6TQSyGxwI1EblZZEsQ0vMlAFQflz0v0= github.com/mitchellh/go-wordwrap v1.0.1/go.mod h1:R62XHJLzvMFRBbcrT7m7WgmE1eOyTSsCt+hzestvNj0= +github.com/moby/docker-image-spec v1.3.1 h1:jMKff3w6PgbfSa69GfNg+zN/XLhfXJGnEx3Nl2EsFP0= +github.com/moby/docker-image-spec v1.3.1/go.mod h1:eKmb5VW8vQEh/BAr2yvVNvuiJuY6UIocYsFu/DxxRpo= github.com/moby/spdystream v0.2.0 h1:cjW1zVyyoiM0T7b6UoySUFqzXMoqRckQtXwGPiBhOM8= github.com/moby/spdystream v0.2.0/go.mod h1:f7i0iNDQJ059oMTcWxx8MA/zKFIuD/lY+0GqbN2Wy8c= github.com/moby/term v0.5.0 h1:xt8Q1nalod/v7BqbG21f8mQPqH+xAaC9C3N3wfWbVP0= diff --git a/internal/controller/pkg/manager/fuzz_test.go b/internal/controller/pkg/manager/fuzz_test.go index 48d0d789d..e32941977 100644 --- a/internal/controller/pkg/manager/fuzz_test.go +++ b/internal/controller/pkg/manager/fuzz_test.go @@ -38,7 +38,7 @@ func FuzzPackageRevision(f *testing.F) { MockHead: fake.NewMockHeadFn(nil, errors.New("boom")), } r := NewPackageRevisioner(fetcher) - _, _ = r.Revision(context.Background(), pkg) + _, _ = r.Revision(context.Background(), pkg, "") n, err := ff.GetString() if err != nil { t.Skip() diff --git a/internal/controller/pkg/manager/reconciler.go b/internal/controller/pkg/manager/reconciler.go index 0b4e6c055..fb0831b81 100644 --- a/internal/controller/pkg/manager/reconciler.go +++ b/internal/controller/pkg/manager/reconciler.go @@ -19,6 +19,7 @@ package manager import ( "context" + "fmt" "math" "reflect" "strings" @@ -26,10 +27,12 @@ import ( corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/reconcile" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" @@ -41,6 +44,7 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/resource" v1 "github.com/crossplane/crossplane/apis/pkg/v1" + "github.com/crossplane/crossplane/apis/pkg/v1beta1" "github.com/crossplane/crossplane/internal/controller/pkg/controller" "github.com/crossplane/crossplane/internal/xpkg" ) @@ -69,6 +73,7 @@ const ( errUnpack = "cannot unpack package" errApplyPackageRevision = "cannot apply package revision" errGCPackageRevision = "cannot garbage collect old package revision" + errGetPullConfig = "cannot get image pull secret from config" errUpdateStatus = "cannot update package status" errUpdateInactivePackageRevision = "cannot update inactive package revision" @@ -88,6 +93,7 @@ const ( reasonGarbageCollect event.Reason = "GarbageCollect" reasonInstall event.Reason = "InstallPackageRevision" reasonPaused event.Reason = "ReconciliationPaused" + reasonImageConfig event.Reason = "ImageConfigSelection" ) // ReconcilerOption is used to configure the Reconciler. @@ -122,6 +128,13 @@ func WithRevisioner(d Revisioner) ReconcilerOption { } } +// WithConfigStore specifies the image config store to use. +func WithConfigStore(c xpkg.ConfigStore) ReconcilerOption { + return func(r *Reconciler) { + r.config = c + } +} + // WithLogger specifies how the Reconciler should log messages. func WithLogger(log logging.Logger) ReconcilerOption { return func(r *Reconciler) { @@ -140,6 +153,7 @@ func WithRecorder(er event.Recorder) ReconcilerOption { type Reconciler struct { client resource.ClientApplicator pkg Revisioner + config xpkg.ConfigStore log logging.Logger record event.Recorder @@ -164,12 +178,14 @@ func SetupProvider(mgr ctrl.Manager, o controller.Options) error { return errors.Wrap(err, errBuildFetcher) } + log := o.Logger.WithValues("controller", name) opts := []ReconcilerOption{ WithNewPackageFn(np), WithNewPackageRevisionFn(nr), WithNewPackageRevisionListFn(nrl), WithRevisioner(NewPackageRevisioner(f, WithDefaultRegistry(o.DefaultRegistry))), - WithLogger(o.Logger.WithValues("controller", name)), + WithConfigStore(xpkg.NewImageConfigStore(mgr.GetClient())), + WithLogger(log), WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))), } @@ -177,6 +193,7 @@ func SetupProvider(mgr ctrl.Manager, o controller.Options) error { Named(name). For(&v1.Provider{}). Owns(&v1.ProviderRevision{}). + Watches(&v1beta1.ImageConfig{}, enqueueProvidersForImageConfig(mgr.GetClient(), log)). WithOptions(o.ForControllerRuntime()). Complete(ratelimiter.NewReconciler(name, errors.WithSilentRequeueOnConflict(NewReconciler(mgr, opts...)), o.GlobalRateLimiter)) } @@ -197,12 +214,14 @@ func SetupConfiguration(mgr ctrl.Manager, o controller.Options) error { return errors.Wrap(err, "cannot build fetcher") } + log := o.Logger.WithValues("controller", name) r := NewReconciler(mgr, WithNewPackageFn(np), WithNewPackageRevisionFn(nr), WithNewPackageRevisionListFn(nrl), WithRevisioner(NewPackageRevisioner(fetcher, WithDefaultRegistry(o.DefaultRegistry))), - WithLogger(o.Logger.WithValues("controller", name)), + WithConfigStore(xpkg.NewImageConfigStore(mgr.GetClient())), + WithLogger(log), WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))), ) @@ -210,6 +229,7 @@ func SetupConfiguration(mgr ctrl.Manager, o controller.Options) error { Named(name). For(&v1.Configuration{}). Owns(&v1.ConfigurationRevision{}). + Watches(&v1beta1.ImageConfig{}, enqueueConfigurationsForImageConfig(mgr.GetClient(), log)). WithOptions(o.ForControllerRuntime()). Complete(ratelimiter.NewReconciler(name, errors.WithSilentRequeueOnConflict(r), o.GlobalRateLimiter)) } @@ -230,12 +250,14 @@ func SetupFunction(mgr ctrl.Manager, o controller.Options) error { return errors.Wrap(err, errBuildFetcher) } + log := o.Logger.WithValues("controller", name) opts := []ReconcilerOption{ WithNewPackageFn(np), WithNewPackageRevisionFn(nr), WithNewPackageRevisionListFn(nrl), WithRevisioner(NewPackageRevisioner(f, WithDefaultRegistry(o.DefaultRegistry))), - WithLogger(o.Logger.WithValues("controller", name)), + WithConfigStore(xpkg.NewImageConfigStore(mgr.GetClient())), + WithLogger(log), WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))), } @@ -243,6 +265,7 @@ func SetupFunction(mgr ctrl.Manager, o controller.Options) error { Named(name). For(&v1.Function{}). Owns(&v1.FunctionRevision{}). + Watches(&v1beta1.ImageConfig{}, enqueueFunctionsForImageConfig(mgr.GetClient(), log)). WithOptions(o.ForControllerRuntime()). Complete(ratelimiter.NewReconciler(name, errors.WithSilentRequeueOnConflict(NewReconciler(mgr, opts...)), o.GlobalRateLimiter)) } @@ -303,7 +326,22 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco return reconcile.Result{}, err } - revisionName, err := r.pkg.Revision(ctx, p) + imageConfig, pullSecretFromConfig, err := r.config.PullSecretFor(ctx, p.GetSource()) + if err != nil { + err = errors.Wrap(err, errGetPullConfig) + p.SetConditions(v1.Unpacking().WithMessage(err.Error())) + _ = r.client.Status().Update(ctx, p) + + r.record.Event(p, event.Warning(reasonImageConfig, err)) + + return reconcile.Result{}, err + } + + var secrets []string + if pullSecretFromConfig != "" { + secrets = append(secrets, pullSecretFromConfig) + } + revisionName, err := r.pkg.Revision(ctx, p, secrets...) if err != nil { err = errors.Wrap(err, errUnpack) p.SetConditions(v1.Unpacking().WithMessage(err.Error())) @@ -407,6 +445,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco r.record.Event(p, event.Warning(reasonInstall, errors.New(errUnknownPackageRevisionHealth))) } + if pr.GetUID() == "" && imageConfig != "" { + // We only record this event if the revision is new, as we don't want to + // spam the user with events if the revision already exists. + log.Debug("Selected pull secret from image config store", "image", p.GetSource(), "imageConfig", imageConfig, "pullSecret", pullSecretFromConfig) + r.record.Event(p, event.Normal(reasonImageConfig, fmt.Sprintf("Selected pullSecret %q from ImageConfig %q for registry authentication", pullSecretFromConfig, imageConfig))) + } + // Create the non-existent package revision. pr.SetName(revisionName) pr.SetLabels(map[string]string{v1.LabelParentPackage: p.GetName()}) @@ -471,3 +516,96 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco // will match the health of the old revision until the next reconcile. return pullBasedRequeue(p.GetPackagePullPolicy()), errors.Wrap(r.client.Status().Update(ctx, p), errUpdateStatus) } + +func enqueueProvidersForImageConfig(kube client.Client, log logging.Logger) handler.EventHandler { + return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request { + ic, ok := o.(*v1beta1.ImageConfig) + if !ok { + return nil + } + // We only care about ImageConfigs that have a pull secret. + if ic.Spec.Registry == nil || ic.Spec.Registry.Authentication == nil || ic.Spec.Registry.Authentication.PullSecretRef.Name == "" { + return nil + } + // Enqueue all Providers matching the prefixes in the ImageConfig. + l := &v1.ProviderList{} + if err := kube.List(ctx, l); err != nil { + // Nothing we can do, except logging, if we can't list Providers. + log.Debug("Cannot list providers while attempting to enqueue from ImageConfig", "error", err) + return nil + } + + var matches []reconcile.Request + for _, p := range l.Items { + for _, m := range ic.Spec.MatchImages { + if strings.HasPrefix(p.GetSource(), m.Prefix) { + log.Debug("Enqueuing provider for image config", "provider", p.Name, "imageConfig", ic.Name) + matches = append(matches, reconcile.Request{NamespacedName: types.NamespacedName{Name: p.Name}}) + } + } + } + return matches + }) +} + +func enqueueConfigurationsForImageConfig(kube client.Client, log logging.Logger) handler.EventHandler { + return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request { + ic, ok := o.(*v1beta1.ImageConfig) + if !ok { + return nil + } + // We only care about ImageConfigs that have a pull secret. + if ic.Spec.Registry == nil || ic.Spec.Registry.Authentication == nil || ic.Spec.Registry.Authentication.PullSecretRef.Name == "" { + return nil + } + // Enqueue all Configurations matching the prefixes in the ImageConfig. + l := &v1.ConfigurationList{} + if err := kube.List(ctx, l); err != nil { + // Nothing we can do, except logging, if we can't list Configurations. + log.Debug("Cannot list configurations while attempting to enqueue from ImageConfig", "error", err) + return nil + } + + var matches []reconcile.Request + for _, c := range l.Items { + for _, m := range ic.Spec.MatchImages { + if strings.HasPrefix(c.GetSource(), m.Prefix) { + log.Debug("Enqueuing configuration for image config", "configuration", c.Name, "imageConfig", ic.Name) + matches = append(matches, reconcile.Request{NamespacedName: types.NamespacedName{Name: c.Name}}) + } + } + } + return matches + }) +} + +func enqueueFunctionsForImageConfig(kube client.Client, log logging.Logger) handler.EventHandler { + return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request { + ic, ok := o.(*v1beta1.ImageConfig) + if !ok { + return nil + } + // We only care about ImageConfigs that have a pull secret. + if ic.Spec.Registry == nil || ic.Spec.Registry.Authentication == nil || ic.Spec.Registry.Authentication.PullSecretRef.Name == "" { + return nil + } + // Enqueue all Functions matching the prefixes in the ImageConfig. + l := &v1.FunctionList{} + if err := kube.List(ctx, l); err != nil { + // Nothing we can do, except logging, if we can't list Functions. + log.Debug("Cannot list functions while attempting to enqueue from ImageConfig", "error", err) + return nil + } + + var matches []reconcile.Request + for _, fn := range l.Items { + for _, m := range ic.Spec.MatchImages { + if strings.HasPrefix(fn.GetSource(), m.Prefix) { + log.Debug("Enqueuing function for image config", "function", fn.Name, "imageConfig", ic.Name) + matches = append(matches, reconcile.Request{NamespacedName: types.NamespacedName{Name: fn.Name}}) + } + } + } + return matches + }) +} diff --git a/internal/controller/pkg/manager/reconciler_test.go b/internal/controller/pkg/manager/reconciler_test.go index 2313bb1d7..17de390fd 100644 --- a/internal/controller/pkg/manager/reconciler_test.go +++ b/internal/controller/pkg/manager/reconciler_test.go @@ -40,6 +40,7 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/test" v1 "github.com/crossplane/crossplane/apis/pkg/v1" + "github.com/crossplane/crossplane/internal/xpkg/fake" ) var _ Revisioner = &MockRevisioner{} @@ -54,7 +55,7 @@ func NewMockRevisionFn(hash string, err error) func() (string, error) { } } -func (m *MockRevisioner) Revision(context.Context, v1.Package) (string, error) { +func (m *MockRevisioner) Revision(context.Context, v1.Package, ...string) (string, error) { return m.MockRevision() } @@ -132,6 +133,44 @@ func TestReconcile(t *testing.T) { err: errors.Wrap(errBoom, errListRevisions), }, }, + "ErrGetPullConfig": { + reason: "We should return an error if getting the pull secret from image configs.", + args: args{ + req: reconcile.Request{NamespacedName: types.NamespacedName{Name: "test"}}, + rec: &Reconciler{ + newPackage: func() v1.Package { return &v1.Configuration{} }, + newPackageRevisionList: func() v1.PackageRevisionList { return &v1.ConfigurationRevisionList{} }, + client: resource.ClientApplicator{ + Client: &test.MockClient{ + MockGet: test.NewMockGetFn(nil), + MockList: test.NewMockListFn(kerrors.NewNotFound(schema.GroupResource{}, "")), + MockStatusUpdate: test.NewMockSubResourceUpdateFn(nil, func(o client.Object) error { + want := &v1.Configuration{} + want.SetConditions(v1.Unpacking().WithMessage(errors.Wrap(errBoom, errGetPullConfig).Error())) + if diff := cmp.Diff(want, o); diff != "" { + t.Errorf("-want, +got:\n%s", diff) + } + return nil + }), + }, + Applicator: resource.ApplyFn(func(_ context.Context, _ client.Object, _ ...resource.ApplyOption) error { + return nil + }), + }, + log: testLog, + record: event.NewNopRecorder(), + pkg: &MockRevisioner{ + MockRevision: NewMockRevisionFn("", errBoom), + }, + config: &fake.MockConfigStore{ + MockPullSecretFor: fake.NewMockConfigStorePullSecretForFn("", "", errBoom), + }, + }, + }, + want: want{ + err: errors.Wrap(errBoom, errGetPullConfig), + }, + }, "ErrFetchRevision": { reason: "We should return an error if fetching the revision for a package fails.", args: args{ @@ -161,6 +200,9 @@ func TestReconcile(t *testing.T) { pkg: &MockRevisioner{ MockRevision: NewMockRevisionFn("", errBoom), }, + config: &fake.MockConfigStore{ + MockPullSecretFor: fake.NewMockConfigStorePullSecretForFn("", "", nil), + }, }, }, want: want{ @@ -206,6 +248,9 @@ func TestReconcile(t *testing.T) { pkg: &MockRevisioner{ MockRevision: NewMockRevisionFn("test-1234567", nil), }, + config: &fake.MockConfigStore{ + MockPullSecretFor: fake.NewMockConfigStorePullSecretForFn("", "", nil), + }, log: testLog, record: event.NewNopRecorder(), }, @@ -255,6 +300,9 @@ func TestReconcile(t *testing.T) { pkg: &MockRevisioner{ MockRevision: NewMockRevisionFn("test-1234567", nil), }, + config: &fake.MockConfigStore{ + MockPullSecretFor: fake.NewMockConfigStorePullSecretForFn("", "", nil), + }, log: testLog, record: event.NewNopRecorder(), }, @@ -302,6 +350,9 @@ func TestReconcile(t *testing.T) { pkg: &MockRevisioner{ MockRevision: NewMockRevisionFn("test-1234567", nil), }, + config: &fake.MockConfigStore{ + MockPullSecretFor: fake.NewMockConfigStorePullSecretForFn("", "", nil), + }, log: testLog, record: event.NewNopRecorder(), }, @@ -360,6 +411,9 @@ func TestReconcile(t *testing.T) { pkg: &MockRevisioner{ MockRevision: NewMockRevisionFn("test-1234567", nil), }, + config: &fake.MockConfigStore{ + MockPullSecretFor: fake.NewMockConfigStorePullSecretForFn("", "", nil), + }, log: testLog, record: event.NewNopRecorder(), }, @@ -438,6 +492,9 @@ func TestReconcile(t *testing.T) { pkg: &MockRevisioner{ MockRevision: NewMockRevisionFn("test-1234567", nil), }, + config: &fake.MockConfigStore{ + MockPullSecretFor: fake.NewMockConfigStorePullSecretForFn("", "", nil), + }, log: testLog, record: event.NewNopRecorder(), }, @@ -497,6 +554,9 @@ func TestReconcile(t *testing.T) { pkg: &MockRevisioner{ MockRevision: NewMockRevisionFn("test-1234567", nil), }, + config: &fake.MockConfigStore{ + MockPullSecretFor: fake.NewMockConfigStorePullSecretForFn("", "", nil), + }, log: testLog, record: event.NewNopRecorder(), }, @@ -557,6 +617,9 @@ func TestReconcile(t *testing.T) { pkg: &MockRevisioner{ MockRevision: NewMockRevisionFn("test-1234567", nil), }, + config: &fake.MockConfigStore{ + MockPullSecretFor: fake.NewMockConfigStorePullSecretForFn("", "", nil), + }, log: testLog, record: event.NewNopRecorder(), }, @@ -654,6 +717,9 @@ func TestReconcile(t *testing.T) { pkg: &MockRevisioner{ MockRevision: NewMockRevisionFn("test-1234567", nil), }, + config: &fake.MockConfigStore{ + MockPullSecretFor: fake.NewMockConfigStorePullSecretForFn("", "", nil), + }, log: testLog, record: event.NewNopRecorder(), }, @@ -733,6 +799,9 @@ func TestReconcile(t *testing.T) { pkg: &MockRevisioner{ MockRevision: NewMockRevisionFn("test-1234567", nil), }, + config: &fake.MockConfigStore{ + MockPullSecretFor: fake.NewMockConfigStorePullSecretForFn("", "", nil), + }, log: testLog, record: event.NewNopRecorder(), }, @@ -780,6 +849,9 @@ func TestReconcile(t *testing.T) { pkg: &MockRevisioner{ MockRevision: NewMockRevisionFn("test-1234567", nil), }, + config: &fake.MockConfigStore{ + MockPullSecretFor: fake.NewMockConfigStorePullSecretForFn("", "", nil), + }, log: testLog, record: event.NewNopRecorder(), }, @@ -828,6 +900,9 @@ func TestReconcile(t *testing.T) { pkg: &MockRevisioner{ MockRevision: NewMockRevisionFn("test-1234567", nil), }, + config: &fake.MockConfigStore{ + MockPullSecretFor: fake.NewMockConfigStorePullSecretForFn("", "", nil), + }, log: testLog, record: event.NewNopRecorder(), }, diff --git a/internal/controller/pkg/manager/revisioner.go b/internal/controller/pkg/manager/revisioner.go index 7ed09cf8a..7ca0e8ac7 100644 --- a/internal/controller/pkg/manager/revisioner.go +++ b/internal/controller/pkg/manager/revisioner.go @@ -35,7 +35,7 @@ const ( // Revisioner extracts a revision name for a package source. type Revisioner interface { - Revision(ctx context.Context, p v1.Package) (string, error) + Revision(ctx context.Context, p v1.Package, extraPullSecrets ...string) (string, error) } // PackageRevisioner extracts a revision name for a package source. @@ -66,7 +66,7 @@ func NewPackageRevisioner(fetcher xpkg.Fetcher, opts ...PackageRevisionerOption) } // Revision extracts a revision name for a package source. -func (r *PackageRevisioner) Revision(ctx context.Context, p v1.Package) (string, error) { +func (r *PackageRevisioner) Revision(ctx context.Context, p v1.Package, extraPullSecrets ...string) (string, error) { pullPolicy := p.GetPackagePullPolicy() if pullPolicy != nil && *pullPolicy == corev1.PullNever { return xpkg.FriendlyID(p.GetName(), p.GetSource()), nil @@ -80,7 +80,12 @@ func (r *PackageRevisioner) Revision(ctx context.Context, p v1.Package) (string, if err != nil { return "", errors.Wrap(err, errBadReference) } - d, err := r.fetcher.Head(ctx, ref, v1.RefNames(p.GetPackagePullSecrets())...) + + ps := v1.RefNames(p.GetPackagePullSecrets()) + if len(extraPullSecrets) > 0 { + ps = append(ps, extraPullSecrets...) + } + d, err := r.fetcher.Head(ctx, ref, ps...) if err != nil || d == nil { return "", errors.Wrap(err, errFetchPackage) } @@ -96,6 +101,6 @@ func NewNopRevisioner() *NopRevisioner { } // Revision returns an empty revision name and no error. -func (d *NopRevisioner) Revision(context.Context, v1.Package) (string, error) { +func (d *NopRevisioner) Revision(context.Context, v1.Package, ...string) (string, error) { return "", nil } diff --git a/internal/controller/pkg/manager/revisioner_test.go b/internal/controller/pkg/manager/revisioner_test.go index 93807eea2..015483b3a 100644 --- a/internal/controller/pkg/manager/revisioner_test.go +++ b/internal/controller/pkg/manager/revisioner_test.go @@ -38,8 +38,9 @@ func TestPackageRevisioner(t *testing.T) { pullIfNotPresent := corev1.PullIfNotPresent type args struct { - f xpkg.Fetcher - pkg v1.Package + f xpkg.Fetcher + pkg v1.Package + pullSecretFromConfig string } type want struct { @@ -134,7 +135,7 @@ func TestPackageRevisioner(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { r := NewPackageRevisioner(tc.args.f) - h, err := r.Revision(context.TODO(), tc.args.pkg) + h, err := r.Revision(context.TODO(), tc.args.pkg, tc.args.pullSecretFromConfig) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("\n%s\nr.Name(...): -want error, +got error:\n%s", tc.reason, diff) diff --git a/internal/controller/pkg/resolver/reconciler.go b/internal/controller/pkg/resolver/reconciler.go index c0bc15f9d..8771e164b 100644 --- a/internal/controller/pkg/resolver/reconciler.go +++ b/internal/controller/pkg/resolver/reconciler.go @@ -30,6 +30,7 @@ import ( "k8s.io/client-go/kubernetes" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -52,6 +53,7 @@ const ( ) const ( + lockName = "lock" finalizer = "lock.pkg.crossplane.io" errGetLock = "cannot get package lock" @@ -62,6 +64,8 @@ const ( errFmtMissingDependency = "missing package (%s) is not a dependency" errInvalidConstraint = "version constraint on dependency is invalid" errInvalidDependency = "dependency package is not valid" + errFindDependency = "cannot find dependency version" + errGetPullConfig = "cannot get image pull secret from config" errFetchTags = "cannot fetch dependency package tags" errNoValidVersion = "cannot find a valid version for package constraints" errFmtNoValidVersion = "dependency (%s) does not have version in constraints (%s)" @@ -100,6 +104,13 @@ func WithFetcher(f xpkg.Fetcher) ReconcilerOption { } } +// WithConfigStore specifies how the Reconciler should access image config store. +func WithConfigStore(c xpkg.ConfigStore) ReconcilerOption { + return func(r *Reconciler) { + r.config = c + } +} + // WithDefaultRegistry sets the default registry to use. func WithDefaultRegistry(registry string) ReconcilerOption { return func(r *Reconciler) { @@ -114,6 +125,7 @@ type Reconciler struct { lock resource.Finalizer newDag dag.NewDAGFn fetcher xpkg.Fetcher + config xpkg.ConfigStore registry string } @@ -134,6 +146,7 @@ func Setup(mgr ctrl.Manager, o controller.Options) error { WithLogger(o.Logger.WithValues("controller", name)), WithFetcher(f), WithDefaultRegistry(o.DefaultRegistry), + WithConfigStore(xpkg.NewImageConfigStore(mgr.GetClient())), ) return ctrl.NewControllerManagedBy(mgr). @@ -141,6 +154,23 @@ func Setup(mgr ctrl.Manager, o controller.Options) error { For(&v1beta1.Lock{}). Owns(&v1.ConfigurationRevision{}). Owns(&v1.ProviderRevision{}). + Watches(&v1beta1.ImageConfig{}, handler.EnqueueRequestsFromMapFunc(func(_ context.Context, o client.Object) []reconcile.Request { + ic, ok := o.(*v1beta1.ImageConfig) + if !ok { + return nil + } + // We only care about ImageConfigs that have a pull secret. + if ic.Spec.Registry == nil || ic.Spec.Registry.Authentication == nil || ic.Spec.Registry.Authentication.PullSecretRef.Name == "" { + return nil + } + // Ideally we should enqueue only if the ImageConfig applies to a + // package in the Lock which would require getting/parsing the Lock + // and checking the source of each package against the prefixes in + // the ImageConfig. However, this is a bit more complex than needed, + // and we don't expect to have many ImageConfigs so we just enqueue + // for all ImageConfigs with a pull secret. + return []reconcile.Request{{NamespacedName: client.ObjectKey{Name: lockName}}} + })). WithOptions(o.ForControllerRuntime()). Complete(ratelimiter.NewReconciler(name, errors.WithSilentRequeueOnConflict(r), o.GlobalRateLimiter)) } @@ -246,10 +276,22 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco return reconcile.Result{Requeue: false}, nil } + ic, ps, err := r.config.PullSecretFor(ctx, ref.String()) + if err != nil { + log.Info("cannot get pull secret from image config store", "error", err) + return reconcile.Result{Requeue: false}, errors.Wrap(err, errGetPullConfig) + } + + var s []string + if ps != "" { + log.Debug("Selected pull secret from image config store", "image", ref.String(), "imageConfig", ic, "pullSecret", ps) + s = append(s, ps) + } + // NOTE(hasheddan): we will be unable to fetch tags for private // dependencies because we do not attach any secrets. Consider copying // secrets from parent dependencies. - tags, err := r.fetcher.Tags(ctx, ref) + tags, err := r.fetcher.Tags(ctx, ref, s...) if err != nil { log.Debug(errFetchTags, "error", err) return reconcile.Result{}, errors.Wrap(err, errFetchTags) diff --git a/internal/controller/pkg/resolver/reconciler_test.go b/internal/controller/pkg/resolver/reconciler_test.go index b8cfa7efd..b99749627 100644 --- a/internal/controller/pkg/resolver/reconciler_test.go +++ b/internal/controller/pkg/resolver/reconciler_test.go @@ -297,6 +297,56 @@ func TestReconcile(t *testing.T) { r: reconcile.Result{Requeue: false}, }, }, + "ErrorGetPullSecretFromImageConfig": { + reason: "We should return an error if fail to get pull secret from configs.", + args: args{ + mgr: &fake.Manager{ + Client: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(o client.Object) error { + // Populate package list so we attempt + // reconciliation. This is overridden by the mock + // DAG. + l := o.(*v1beta1.Lock) + l.Packages = append(l.Packages, v1beta1.LockPackage{ + Name: "cool-package", + Type: v1beta1.ProviderPackageType, + Source: "cool-repo/cool-image", + Version: "v0.0.1", + }) + return nil + }), + MockUpdate: test.NewMockUpdateFn(nil), + }, + }, + req: reconcile.Request{NamespacedName: types.NamespacedName{Name: "test"}}, + rec: []ReconcilerOption{ + WithNewDagFn(func() dag.DAG { + return &fakedag.MockDag{ + MockInit: func(_ []dag.Node) ([]dag.Node, error) { + return []dag.Node{ + &v1beta1.Dependency{ + Package: "registry1.com/acme-co/configuration-foo", + Constraints: "v0.0.1", + }, + }, nil + }, + MockSort: func() ([]string, error) { + return nil, nil + }, + } + }), + WithFetcher(&fakexpkg.MockFetcher{ + MockTags: fakexpkg.NewMockTagsFn(nil, errBoom), + }), + WithConfigStore(&fakexpkg.MockConfigStore{ + MockPullSecretFor: fakexpkg.NewMockConfigStorePullSecretForFn("", "", errBoom), + }), + }, + }, + want: want{ + err: errors.Wrap(errBoom, errGetPullConfig), + }, + }, "ErrorFetchTags": { reason: "We should return an error if fail to fetch tags to account for network issues.", args: args{ @@ -338,6 +388,9 @@ func TestReconcile(t *testing.T) { WithFetcher(&fakexpkg.MockFetcher{ MockTags: fakexpkg.NewMockTagsFn(nil, errBoom), }), + WithConfigStore(&fakexpkg.MockConfigStore{ + MockPullSecretFor: fakexpkg.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -385,6 +438,9 @@ func TestReconcile(t *testing.T) { WithFetcher(&fakexpkg.MockFetcher{ MockTags: fakexpkg.NewMockTagsFn([]string{"v0.2.0", "v0.3.0", "v1.0.0"}, nil), }), + WithConfigStore(&fakexpkg.MockConfigStore{ + MockPullSecretFor: fakexpkg.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -434,6 +490,9 @@ func TestReconcile(t *testing.T) { WithFetcher(&fakexpkg.MockFetcher{ MockTags: fakexpkg.NewMockTagsFn([]string{"v0.2.0", "v0.3.0", "v1.0.0", "v1.2.0"}, nil), }), + WithConfigStore(&fakexpkg.MockConfigStore{ + MockPullSecretFor: fakexpkg.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -483,6 +542,9 @@ func TestReconcile(t *testing.T) { WithFetcher(&fakexpkg.MockFetcher{ MockTags: fakexpkg.NewMockTagsFn([]string{"v0.2.0", "v0.3.0", "v1.0.0", "v1.2.0"}, nil), }), + WithConfigStore(&fakexpkg.MockConfigStore{ + MockPullSecretFor: fakexpkg.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ diff --git a/internal/controller/pkg/revision/imageback.go b/internal/controller/pkg/revision/imageback.go index 81500cf81..00299cc65 100644 --- a/internal/controller/pkg/revision/imageback.go +++ b/internal/controller/pkg/revision/imageback.go @@ -98,7 +98,11 @@ func (i *ImageBackend) Init(ctx context.Context, bo ...parser.BackendOption) (io return nil, errors.Wrap(err, errBadReference) } // Fetch image from registry. - img, err := i.fetcher.Fetch(ctx, ref, v1.RefNames(n.pr.GetPackagePullSecrets())...) + ps := v1.RefNames(n.pr.GetPackagePullSecrets()) + if n.pullSecretFromConfig != "" { + ps = append(ps, n.pullSecretFromConfig) + } + img, err := i.fetcher.Fetch(ctx, ref, ps...) if err != nil { return nil, errors.Wrap(err, errFetchPackage) } @@ -178,7 +182,8 @@ func (i *ImageBackend) Init(ctx context.Context, bo ...parser.BackendOption) (io // options. // NOTE(hasheddan): see usage in ImageBackend Init() for reasoning. type nestedBackend struct { - pr v1.PackageRevision + pr v1.PackageRevision + pullSecretFromConfig string } // Init is a nop because nestedBackend does not actually meant to act as a @@ -197,3 +202,14 @@ func PackageRevision(pr v1.PackageRevision) parser.BackendOption { i.pr = pr } } + +// PullSecretFromConfig sets the image config pull secret for ImageBackend. +func PullSecretFromConfig(secret string) parser.BackendOption { + return func(p parser.Backend) { + i, ok := p.(*nestedBackend) + if !ok { + return + } + i.pullSecretFromConfig = secret + } +} diff --git a/internal/controller/pkg/revision/reconciler.go b/internal/controller/pkg/revision/reconciler.go index 1d6d04dcd..5dc66ee46 100644 --- a/internal/controller/pkg/revision/reconciler.go +++ b/internal/controller/pkg/revision/reconciler.go @@ -19,6 +19,7 @@ package revision import ( "context" + "fmt" "io" "sort" "strings" @@ -32,6 +33,7 @@ import ( "k8s.io/client-go/kubernetes" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -76,6 +78,8 @@ const ( errAddFinalizer = "cannot add package revision finalizer" errRemoveFinalizer = "cannot remove package revision finalizer" + errGetPullConfig = "cannot get image pull secret from config" + errDeactivateRevision = "cannot deactivate package revision" errInitParserBackend = "cannot initialize parser backend" @@ -114,6 +118,7 @@ const ( // Event reasons. const ( + reasonImageConfig event.Reason = "ImageConfigSelection" reasonParse event.Reason = "ParsePackage" reasonLint event.Reason = "LintPackage" reasonDependencies event.Reason = "ResolveDependencies" @@ -206,6 +211,14 @@ func WithParserBackend(p parser.Backend) ReconcilerOption { } } +// WithConfigStore specifies the ConfigStore to use for fetching image +// configurations. +func WithConfigStore(c xpkg.ConfigStore) ReconcilerOption { + return func(r *Reconciler) { + r.config = c + } +} + // WithLinter specifies how the Reconciler should lint a package. func WithLinter(l parser.Linter) ReconcilerOption { return func(r *Reconciler) { @@ -261,6 +274,7 @@ type Reconciler struct { linter parser.Linter versioner version.Operations backend parser.Backend + config xpkg.ConfigStore log logging.Logger record event.Recorder features *feature.Flags @@ -293,6 +307,7 @@ func SetupProviderRevision(mgr ctrl.Manager, o controller.Options) error { return errors.Wrap(err, errCannotBuildFetcher) } + log := o.Logger.WithValues("controller", name) cb := ctrl.NewControllerManagedBy(mgr). Named(name). For(&v1.ProviderRevision{}). @@ -302,7 +317,8 @@ func SetupProviderRevision(mgr ctrl.Manager, o controller.Options) error { Owns(&corev1.ServiceAccount{}). Watches(&v1alpha1.ControllerConfig{}, &EnqueueRequestForReferencingProviderRevisions{ client: mgr.GetClient(), - }) + }). + Watches(&v1beta1.ImageConfig{}, enqueueProviderRevisionsForImageConfig(mgr.GetClient(), log)) ro := []ReconcilerOption{ WithCache(o.Cache), @@ -311,8 +327,9 @@ func SetupProviderRevision(mgr ctrl.Manager, o controller.Options) error { WithNewPackageRevisionFn(nr), WithParser(parser.New(metaScheme, objScheme)), WithParserBackend(NewImageBackend(fetcher, WithDefaultRegistry(o.DefaultRegistry))), + WithConfigStore(xpkg.NewImageConfigStore(mgr.GetClient())), WithLinter(xpkg.NewProviderLinter()), - WithLogger(o.Logger.WithValues("controller", name)), + WithLogger(log), WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))), WithNamespace(o.Namespace), WithServiceAccount(o.ServiceAccount), @@ -356,6 +373,7 @@ func SetupConfigurationRevision(mgr ctrl.Manager, o controller.Options) error { return errors.Wrap(err, errCannotBuildFetcher) } + log := o.Logger.WithValues("controller", name) r := NewReconciler(mgr, WithCache(o.Cache), WithDependencyManager(NewPackageDependencyManager(mgr.GetClient(), dag.NewMapDag, v1beta1.ConfigurationPackageType)), @@ -363,8 +381,9 @@ func SetupConfigurationRevision(mgr ctrl.Manager, o controller.Options) error { WithEstablisher(NewAPIEstablisher(mgr.GetClient(), o.Namespace)), WithParser(parser.New(metaScheme, objScheme)), WithParserBackend(NewImageBackend(f, WithDefaultRegistry(o.DefaultRegistry))), + WithConfigStore(xpkg.NewImageConfigStore(mgr.GetClient())), WithLinter(xpkg.NewConfigurationLinter()), - WithLogger(o.Logger.WithValues("controller", name)), + WithLogger(log), WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))), WithNamespace(o.Namespace), WithServiceAccount(o.ServiceAccount), @@ -374,6 +393,7 @@ func SetupConfigurationRevision(mgr ctrl.Manager, o controller.Options) error { return ctrl.NewControllerManagedBy(mgr). Named(name). For(&v1.ConfigurationRevision{}). + Watches(&v1beta1.ImageConfig{}, enqueueConfigurationRevisionsForImageConfig(mgr.GetClient(), log)). WithOptions(o.ForControllerRuntime()). Complete(ratelimiter.NewReconciler(name, errors.WithSilentRequeueOnConflict(r), o.GlobalRateLimiter)) } @@ -401,6 +421,7 @@ func SetupFunctionRevision(mgr ctrl.Manager, o controller.Options) error { return errors.Wrap(err, errCannotBuildFetcher) } + log := o.Logger.WithValues("controller", name) cb := ctrl.NewControllerManagedBy(mgr). Named(name). For(&v1.FunctionRevision{}). @@ -410,7 +431,8 @@ func SetupFunctionRevision(mgr ctrl.Manager, o controller.Options) error { Owns(&corev1.ServiceAccount{}). Watches(&v1alpha1.ControllerConfig{}, &EnqueueRequestForReferencingFunctionRevisions{ client: mgr.GetClient(), - }) + }). + Watches(&v1beta1.ImageConfig{}, enqueueFunctionRevisionsForImageConfig(mgr.GetClient(), log)) ro := []ReconcilerOption{ WithCache(o.Cache), @@ -419,8 +441,9 @@ func SetupFunctionRevision(mgr ctrl.Manager, o controller.Options) error { WithNewPackageRevisionFn(nr), WithParser(parser.New(metaScheme, objScheme)), WithParserBackend(NewImageBackend(fetcher, WithDefaultRegistry(o.DefaultRegistry))), + WithConfigStore(xpkg.NewImageConfigStore(mgr.GetClient())), WithLinter(xpkg.NewFunctionLinter()), - WithLogger(o.Logger.WithValues("controller", name)), + WithLogger(log), WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))), WithNamespace(o.Namespace), WithServiceAccount(o.ServiceAccount), @@ -546,6 +569,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco return reconcile.Result{}, err } + imageConfig, pullSecretFromConfig, err := r.config.PullSecretFor(ctx, pr.GetSource()) + if err != nil { + err = errors.Wrap(err, errGetPullConfig) + pr.SetConditions(v1.Unhealthy().WithMessage(err.Error())) + _ = r.client.Status().Update(ctx, pr) + + r.record.Event(pr, event.Warning(reasonImageConfig, err)) + + return reconcile.Result{}, err + } + var runtimeManifestBuilder ManifestBuilder pwr, hasRuntime := pr.(v1.PackageRevisionWithRuntime) if hasRuntime && r.runtimeHook != nil { @@ -562,6 +596,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco return reconcile.Result{}, err } + if pullSecretFromConfig != "" { + opts = append(opts, RuntimeManifestBuilderWithPullSecrets(pullSecretFromConfig)) + } + runtimeManifestBuilder = NewRuntimeManifestBuilder(pwr, r.namespace, opts...) } @@ -657,8 +695,18 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco // If we didn't get a ReadCloser from cache, we need to get it from image. if rc == nil { + bo := []parser.BackendOption{PackageRevision(pr)} + if imageConfig != "" { + bo = append(bo, PullSecretFromConfig(pullSecretFromConfig)) + // We only record this event here, package is not in cache, and + // we are about to fetch the image. This way we avoid emitting + // excessive events in each reconcile, but once per image pull. + log.Debug("Selected pull secret from image config store", "image", pr.GetSource(), "imageConfig", imageConfig, "pullSecret", pullSecretFromConfig) + r.record.Event(pr, event.Normal(reasonImageConfig, fmt.Sprintf("Selected pullSecret %q from ImageConfig %q for registry authentication", pullSecretFromConfig, imageConfig))) + } + // Initialize parser backend to obtain package contents. - imgrc, err := r.backend.Init(ctx, PackageRevision(pr)) + imgrc, err := r.backend.Init(ctx, bo...) if err != nil { err = errors.Wrap(err, errInitParserBackend) pr.SetConditions(v1.Unhealthy().WithMessage(err.Error())) @@ -940,3 +988,96 @@ func (r *Reconciler) runtimeManifestBuilderOptions(ctx context.Context, pwr v1.P return opts, nil } + +func enqueueProviderRevisionsForImageConfig(kube client.Client, log logging.Logger) handler.EventHandler { + return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request { + ic, ok := o.(*v1beta1.ImageConfig) + if !ok { + return nil + } + // We only care about ImageConfigs that have a pull secret. + if ic.Spec.Registry == nil || ic.Spec.Registry.Authentication == nil || ic.Spec.Registry.Authentication.PullSecretRef.Name == "" { + return nil + } + // Enqueue all ProviderRevision matching the prefixes in the ImageConfig. + l := &v1.ProviderRevisionList{} + if err := kube.List(ctx, l); err != nil { + // Nothing we can do, except logging, if we can't list ProviderRevisions. + log.Debug("Cannot list provider revisions while attempting to enqueue from ImageConfig", "error", err) + return nil + } + + var matches []reconcile.Request + for _, p := range l.Items { + for _, m := range ic.Spec.MatchImages { + if strings.HasPrefix(p.GetSource(), m.Prefix) { + log.Debug("Enqueuing provider revision for image config", "providerRevision", p.Name, "imageConfig", ic.Name) + matches = append(matches, reconcile.Request{NamespacedName: types.NamespacedName{Name: p.Name}}) + } + } + } + return matches + }) +} + +func enqueueConfigurationRevisionsForImageConfig(kube client.Client, log logging.Logger) handler.EventHandler { + return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request { + ic, ok := o.(*v1beta1.ImageConfig) + if !ok { + return nil + } + // We only care about ImageConfigs that have a pull secret. + if ic.Spec.Registry == nil || ic.Spec.Registry.Authentication == nil || ic.Spec.Registry.Authentication.PullSecretRef.Name == "" { + return nil + } + // Enqueue all ConfigurationRevision matching the prefixes in the ImageConfig. + l := &v1.ConfigurationRevisionList{} + if err := kube.List(ctx, l); err != nil { + // Nothing we can do, except logging, if we can't list ConfigurationRevisions. + log.Debug("Cannot list configuration revisions while attempting to enqueue from ImageConfig", "error", err) + return nil + } + + var matches []reconcile.Request + for _, p := range l.Items { + for _, m := range ic.Spec.MatchImages { + if strings.HasPrefix(p.GetSource(), m.Prefix) { + log.Debug("Enqueuing configuration revision for image config", "configurationRevision", p.Name, "imageConfig", ic.Name) + matches = append(matches, reconcile.Request{NamespacedName: types.NamespacedName{Name: p.Name}}) + } + } + } + return matches + }) +} + +func enqueueFunctionRevisionsForImageConfig(kube client.Client, log logging.Logger) handler.EventHandler { + return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request { + ic, ok := o.(*v1beta1.ImageConfig) + if !ok { + return nil + } + // We only care about ImageConfigs that have a pull secret. + if ic.Spec.Registry == nil || ic.Spec.Registry.Authentication == nil || ic.Spec.Registry.Authentication.PullSecretRef.Name == "" { + return nil + } + // Enqueue all FunctionRevision matching the prefixes in the ImageConfig. + l := &v1.FunctionRevisionList{} + if err := kube.List(ctx, l); err != nil { + // Nothing we can do, except logging, if we can't list FunctionRevisions. + log.Debug("Cannot list function revisions while attempting to enqueue from ImageConfig", "error", err) + return nil + } + + var matches []reconcile.Request + for _, p := range l.Items { + for _, m := range ic.Spec.MatchImages { + if strings.HasPrefix(p.GetSource(), m.Prefix) { + log.Debug("Enqueuing function revision for image config", "functionRevision", p.Name, "imageConfig", ic.Name) + matches = append(matches, reconcile.Request{NamespacedName: types.NamespacedName{Name: p.Name}}) + } + } + } + return matches + }) +} diff --git a/internal/controller/pkg/revision/reconciler_test.go b/internal/controller/pkg/revision/reconciler_test.go index ea7e2636e..81f6046cd 100644 --- a/internal/controller/pkg/revision/reconciler_test.go +++ b/internal/controller/pkg/revision/reconciler_test.go @@ -394,12 +394,47 @@ func TestReconcile(t *testing.T) { MockGet: xpkgfake.NewMockCacheGetFn(nil, errBoom), MockDelete: xpkgfake.NewMockCacheDeleteFn(nil), }), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ err: errors.Wrap(errBoom, errGetCache), }, }, + "ErrGetPackagePullSecretFromImageConfigs": { + reason: "We should return an error if we cannot get package pull secret from image configs.", + args: args{ + mgr: &fake.Manager{}, + req: reconcile.Request{NamespacedName: types.NamespacedName{Name: "test"}}, + rec: []ReconcilerOption{ + WithNewPackageRevisionFn(func() v1.PackageRevision { return &v1.ConfigurationRevision{} }), + WithClientApplicator(resource.ClientApplicator{ + Client: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(o client.Object) error { + pr := o.(*v1.ConfigurationRevision) + pr.SetGroupVersionKind(v1.ConfigurationRevisionGroupVersionKind) + pr.SetDesiredState(v1.PackageRevisionActive) + return nil + }), + MockStatusUpdate: test.NewMockSubResourceUpdateFn(nil, func(_ client.Object) error { + return nil + }), + }, + }), + WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { + return nil + }}), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", errBoom), + }), + }, + }, + want: want{ + err: errors.Wrap(errBoom, errGetPullConfig), + }, + }, "ErrGetFromCacheFailedDelete": { reason: "We should return an error if package content is in cache, we cannot get it, and we fail to remove it.", args: args{ @@ -429,6 +464,9 @@ func TestReconcile(t *testing.T) { MockGet: xpkgfake.NewMockCacheGetFn(nil, errBoom), MockDelete: xpkgfake.NewMockCacheDeleteFn(errBoom), }), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -476,6 +514,9 @@ func TestReconcile(t *testing.T) { WithCache(&xpkgfake.MockCache{ MockHas: xpkgfake.NewMockCacheHasFn(false), }), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -517,6 +558,9 @@ func TestReconcile(t *testing.T) { MockHas: xpkgfake.NewMockCacheHasFn(false), }), WithParserBackend(&ErrBackend{err: errBoom}), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -559,6 +603,9 @@ func TestReconcile(t *testing.T) { MockHas: xpkgfake.NewMockCacheHasFn(true), MockGet: xpkgfake.NewMockCacheGetFn(io.NopCloser(bytes.NewBuffer(providerBytes)), nil), }), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -602,6 +649,9 @@ func TestReconcile(t *testing.T) { MockHas: xpkgfake.NewMockCacheHasFn(false), MockStore: xpkgfake.NewMockCacheStoreFn(nil), }), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -646,6 +696,9 @@ func TestReconcile(t *testing.T) { MockStore: xpkgfake.NewMockCacheStoreFn(errBoom), MockDelete: xpkgfake.NewMockCacheDeleteFn(nil), }), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -690,6 +743,9 @@ func TestReconcile(t *testing.T) { MockStore: xpkgfake.NewMockCacheStoreFn(errBoom), MockDelete: xpkgfake.NewMockCacheDeleteFn(errBoom), }), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -737,6 +793,9 @@ func TestReconcile(t *testing.T) { return err }, }), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -799,6 +858,9 @@ func TestReconcile(t *testing.T) { MockInConstraints: verfake.NewMockInConstraintsFn(false, errBoom), MockGetVersionString: verfake.NewMockGetVersionStringFn("v0.11.0"), }), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -843,6 +905,9 @@ func TestReconcile(t *testing.T) { MockStore: xpkgfake.NewMockCacheStoreFn(nil), }), WithLinter(&MockLinter{MockLint: NewMockLintFn(nil)}), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -892,6 +957,9 @@ func TestReconcile(t *testing.T) { }, }), WithLinter(&MockLinter{MockLint: NewMockLintFn(nil)}), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -957,6 +1025,9 @@ func TestReconcile(t *testing.T) { }), WithLinter(&MockLinter{MockLint: NewMockLintFn(nil)}), WithVersioner(&verfake.MockVersioner{MockInConstraints: verfake.NewMockInConstraintsFn(true, nil)}), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -1023,6 +1094,9 @@ func TestReconcile(t *testing.T) { }), WithLinter(&MockLinter{MockLint: NewMockLintFn(nil)}), WithVersioner(&verfake.MockVersioner{MockInConstraints: verfake.NewMockInConstraintsFn(true, nil)}), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -1091,6 +1165,9 @@ func TestReconcile(t *testing.T) { }), WithLinter(&MockLinter{MockLint: NewMockLintFn(nil)}), WithVersioner(&verfake.MockVersioner{MockInConstraints: verfake.NewMockInConstraintsFn(true, nil)}), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -1153,6 +1230,9 @@ func TestReconcile(t *testing.T) { }), WithLinter(&MockLinter{MockLint: NewMockLintFn(nil)}), WithVersioner(&verfake.MockVersioner{MockInConstraints: verfake.NewMockInConstraintsFn(true, nil)}), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -1219,6 +1299,9 @@ func TestReconcile(t *testing.T) { }), WithLinter(&MockLinter{MockLint: NewMockLintFn(nil)}), WithVersioner(&verfake.MockVersioner{MockInConstraints: verfake.NewMockInConstraintsFn(false, nil)}), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -1282,6 +1365,9 @@ func TestReconcile(t *testing.T) { }), WithLinter(&MockLinter{MockLint: NewMockLintFn(nil)}), WithVersioner(&verfake.MockVersioner{MockInConstraints: verfake.NewMockInConstraintsFn(true, nil)}), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -1349,6 +1435,9 @@ func TestReconcile(t *testing.T) { return errBoom }, }), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -1414,6 +1503,9 @@ func TestReconcile(t *testing.T) { }), WithLinter(&MockLinter{MockLint: NewMockLintFn(nil)}), WithVersioner(&verfake.MockVersioner{MockInConstraints: verfake.NewMockInConstraintsFn(true, nil)}), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -1477,6 +1569,9 @@ func TestReconcile(t *testing.T) { return nil }}), WithEstablisher(NewMockEstablisher()), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ @@ -1596,6 +1691,9 @@ func TestReconcile(t *testing.T) { }), WithLinter(&MockLinter{MockLint: NewMockLintFn(nil)}), WithVersioner(&verfake.MockVersioner{MockInConstraints: verfake.NewMockInConstraintsFn(true, nil)}), + WithConfigStore(&xpkgfake.MockConfigStore{ + MockPullSecretFor: xpkgfake.NewMockConfigStorePullSecretForFn("", "", nil), + }), }, }, want: want{ diff --git a/internal/controller/pkg/revision/runtime.go b/internal/controller/pkg/revision/runtime.go index e04c99a03..82b4395a7 100644 --- a/internal/controller/pkg/revision/runtime.go +++ b/internal/controller/pkg/revision/runtime.go @@ -101,6 +101,7 @@ type RuntimeManifestBuilder struct { serviceAccountPullSecrets []corev1.LocalObjectReference runtimeConfig *v1beta1.DeploymentRuntimeConfig controllerConfig *v1alpha1.ControllerConfig + pullSecrets []string providerIdentity bool } @@ -131,6 +132,14 @@ func RuntimeManifestBuilderWithServiceAccountPullSecrets(secrets []corev1.LocalO } } +// RuntimeManifestBuilderWithPullSecrets sets the pull secrets to use when +// building the runtime manifests. +func RuntimeManifestBuilderWithPullSecrets(secrets ...string) RuntimeManifestBuilderOption { + return func(b *RuntimeManifestBuilder) { + b.pullSecrets = secrets + } +} + // RuntimeManifestBuilderWithProviderIdentity sets the provider identity flag // to use when building the runtime manifests. func RuntimeManifestBuilderWithProviderIdentity() RuntimeManifestBuilderOption { @@ -194,7 +203,7 @@ func (b *RuntimeManifestBuilder) Deployment(serviceAccount string, overrides ... d = deploymentFromRuntimeConfig(b.runtimeConfig.Spec.DeploymentTemplate) } - var allOverrides []DeploymentOverride + allOverrides := make([]DeploymentOverride, 0, len(overrides)+20) // 20 is just a reasonable guess at the number of overrides we'll add. allOverrides = append(allOverrides, // This will ensure that the runtime container exists and always the // first one. @@ -232,6 +241,10 @@ func (b *RuntimeManifestBuilder) Deployment(serviceAccount string, overrides ... }), ) + for _, s := range b.pullSecrets { + allOverrides = append(allOverrides, DeploymentWithAdditionalPullSecret(corev1.LocalObjectReference{Name: s})) + } + if b.revision.GetPackagePullPolicy() != nil { // If the package pull policy is set, it will override the default // or whatever is set in the runtime config. diff --git a/internal/controller/pkg/revision/runtime_override_options.go b/internal/controller/pkg/revision/runtime_override_options.go index f40cfd7b2..5070b1525 100644 --- a/internal/controller/pkg/revision/runtime_override_options.go +++ b/internal/controller/pkg/revision/runtime_override_options.go @@ -159,6 +159,14 @@ func DeploymentWithImagePullSecrets(secrets []corev1.LocalObjectReference) Deplo } } +// DeploymentWithAdditionalPullSecret adds additional image pull secret to +// a Deployment. +func DeploymentWithAdditionalPullSecret(secret corev1.LocalObjectReference) DeploymentOverride { + return func(d *appsv1.Deployment) { + d.Spec.Template.Spec.ImagePullSecrets = append(d.Spec.Template.Spec.ImagePullSecrets, secret) + } +} + // DeploymentRuntimeWithOptionalImage set the image for the runtime container if // it is unset, e.g. not specified in the DeploymentRuntimeConfig. Note that if // the image was already set, we use it exactly as is (i.e., no default registry). diff --git a/internal/xpkg/config.go b/internal/xpkg/config.go new file mode 100644 index 000000000..54ca83dd3 --- /dev/null +++ b/internal/xpkg/config.go @@ -0,0 +1,95 @@ +package xpkg + +import ( + "context" + "strings" + + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/crossplane/crossplane-runtime/pkg/errors" + + "github.com/crossplane/crossplane/apis/pkg/v1beta1" +) + +const ( + errListImageConfigs = "cannot list ImageConfigs" +) + +// ConfigStore is a store for image configuration. +type ConfigStore interface { + // PullSecretFor returns the name of the selected image config and + // name of the pull secret for a given image. + PullSecretFor(ctx context.Context, image string) (imageConfig, pullSecret string, err error) +} + +// isValidConfig is a function that determines if an ImageConfig is valid while +// finding the best match for an image. +type isValidConfig func(c *v1beta1.ImageConfig) bool + +// ImageConfigStoreOption is an option for image configuration store. +type ImageConfigStoreOption func(*ImageConfigStore) + +// NewImageConfigStore creates a new image configuration store. +func NewImageConfigStore(client client.Client, opts ...ImageConfigStoreOption) ConfigStore { + s := &ImageConfigStore{ + client: client, + } + + for _, opt := range opts { + opt(s) + } + + return s +} + +// ImageConfigStore is a store for image configuration. +type ImageConfigStore struct { + client client.Reader +} + +// PullSecretFor returns the pull secret name for a given image as +// well as the name of the ImageConfig resource that contains the pull secret. +func (s *ImageConfigStore) PullSecretFor(ctx context.Context, image string) (imageConfig, pullSecret string, err error) { + config, err := s.bestMatch(ctx, image, func(c *v1beta1.ImageConfig) bool { + return c.Spec.Registry != nil && c.Spec.Registry.Authentication != nil && c.Spec.Registry.Authentication.PullSecretRef.Name != "" + }) + if err != nil { + return "", "", errors.Wrap(err, errListImageConfigs) + } + + if config == nil { + // No ImageConfig with a pull secret found for this image, this is not + // an error. + return "", "", nil + } + + return config.GetName(), config.Spec.Registry.Authentication.PullSecretRef.Name, nil +} + +// bestMatch finds the best matching ImageConfig for an image based on the +// longest prefix match. +func (s *ImageConfigStore) bestMatch(ctx context.Context, image string, valid isValidConfig) (*v1beta1.ImageConfig, error) { + l := &v1beta1.ImageConfigList{} + + if err := s.client.List(ctx, l); err != nil { + return nil, errors.Wrap(err, "cannot list ImageConfigs") + } + + var config *v1beta1.ImageConfig + var longest int + + for _, c := range l.Items { + if !valid(&c) { + continue + } + + for _, m := range c.Spec.MatchImages { + if strings.HasPrefix(image, m.Prefix) && len(m.Prefix) > longest { + longest = len(m.Prefix) + config = &c + } + } + } + + return config, nil +} diff --git a/internal/xpkg/config_test.go b/internal/xpkg/config_test.go new file mode 100644 index 000000000..c53095b99 --- /dev/null +++ b/internal/xpkg/config_test.go @@ -0,0 +1,438 @@ +package xpkg + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/crossplane/crossplane-runtime/pkg/errors" + "github.com/crossplane/crossplane-runtime/pkg/test" + + "github.com/crossplane/crossplane/apis/pkg/v1beta1" +) + +var errBoom = errors.New("boom") + +func TestImageConfigStoreBestMatch(t *testing.T) { + type args struct { + client client.Client + image string + isValid isValidConfig + } + type want struct { + config *v1beta1.ImageConfig + err error + } + cases := map[string]struct { + args args + want want + }{ + "ErrListConfig": { + args: args{ + image: "registry1.com/acme-co/configuration-foo", + isValid: func(_ *v1beta1.ImageConfig) bool { + return true + }, + client: &test.MockClient{ + MockList: func(_ context.Context, _ client.ObjectList, _ ...client.ListOption) error { + return errBoom + }, + }, + }, + want: want{ + err: errors.Wrap(errBoom, errListImageConfigs), + }, + }, + "SingleConfig": { + args: args{ + image: "registry1.com/acme-co/configuration-foo", + isValid: func(_ *v1beta1.ImageConfig) bool { + return true + }, + client: &test.MockClient{ + MockList: func(_ context.Context, list client.ObjectList, _ ...client.ListOption) error { + *list.(*v1beta1.ImageConfigList) = v1beta1.ImageConfigList{ + Items: []v1beta1.ImageConfig{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com/acme-co"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + }, + } + return nil + }, + }, + }, + want: want{ + config: &v1beta1.ImageConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com/acme-co"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + }, + }, + "SingleConfigMultiPrefix": { + args: args{ + image: "registry1.com/acme-co/configuration-foo", + isValid: func(_ *v1beta1.ImageConfig) bool { + return true + }, + client: &test.MockClient{ + MockList: func(_ context.Context, list client.ObjectList, _ ...client.ListOption) error { + *list.(*v1beta1.ImageConfigList) = v1beta1.ImageConfigList{ + Items: []v1beta1.ImageConfig{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com/some-other"}, + {Prefix: "registry1.com/acme-co"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + }, + } + return nil + }, + }, + }, + want: want{ + config: &v1beta1.ImageConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com/some-other"}, + {Prefix: "registry1.com/acme-co"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + }, + }, + "MultiConfig": { + args: args{ + image: "registry1.com/acme-co/configuration-foo", + isValid: func(_ *v1beta1.ImageConfig) bool { + return true + }, + client: &test.MockClient{ + MockList: func(_ context.Context, list client.ObjectList, _ ...client.ListOption) error { + *list.(*v1beta1.ImageConfigList) = v1beta1.ImageConfigList{ + Items: []v1beta1.ImageConfig{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "registry2", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry2.com/acme-co"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com/acme-co"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + }, + } + return nil + }, + }, + }, + want: want{ + config: &v1beta1.ImageConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com/acme-co"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + }, + }, + "MultiConfigMultiPrefix": { + args: args{ + image: "registry1.com/acme-co/configuration-foo", + isValid: func(_ *v1beta1.ImageConfig) bool { + return true + }, + client: &test.MockClient{ + MockList: func(_ context.Context, list client.ObjectList, _ ...client.ListOption) error { + *list.(*v1beta1.ImageConfigList) = v1beta1.ImageConfigList{ + Items: []v1beta1.ImageConfig{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "registry2", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry2.com/some-other"}, + {Prefix: "registry2.com/acme-co"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com/some-other"}, + {Prefix: "registry1.com/acme-co"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + }, + } + return nil + }, + }, + }, + want: want{ + config: &v1beta1.ImageConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com/some-other"}, + {Prefix: "registry1.com/acme-co"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + }, + }, + "MultiConfigMultiMatchFindsBest": { + args: args{ + image: "registry1.com/acme-co/configuration-foo", + isValid: func(_ *v1beta1.ImageConfig) bool { + return true + }, + client: &test.MockClient{ + MockList: func(_ context.Context, list client.ObjectList, _ ...client.ListOption) error { + *list.(*v1beta1.ImageConfigList) = v1beta1.ImageConfigList{ + Items: []v1beta1.ImageConfig{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1-base", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1-with-org", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com/some-other"}, + {Prefix: "registry1.com/acme-co"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1-full-image-ref", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com/some-other"}, + {Prefix: "registry1.com/acme-co/configuration-foo"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + }, + } + return nil + }, + }, + }, + want: want{ + config: &v1beta1.ImageConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1-full-image-ref", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com/some-other"}, + {Prefix: "registry1.com/acme-co/configuration-foo"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + }, + }, + "SkipsInvalid": { + args: args{ + image: "registry1.com/acme-co/configuration-foo", + isValid: func(c *v1beta1.ImageConfig) bool { + return c.Spec.Registry != nil + }, + client: &test.MockClient{ + MockList: func(_ context.Context, list client.ObjectList, _ ...client.ListOption) error { + *list.(*v1beta1.ImageConfigList) = v1beta1.ImageConfigList{ + Items: []v1beta1.ImageConfig{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1-no-pull-secret", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + // Best match but invalid, no pull secret defined + {Prefix: "registry1.com/acme-co"}, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + }, + } + return nil + }, + }, + }, + want: want{ + config: &v1beta1.ImageConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "registry1", + }, + Spec: v1beta1.ImageConfigSpec{ + MatchImages: []v1beta1.ImageMatch{ + {Prefix: "registry1.com"}, + }, + Registry: &v1beta1.RegistryConfig{ + Authentication: &v1beta1.RegistryAuthentication{ + PullSecretRef: corev1.LocalObjectReference{Name: "test"}, + }, + }, + }, + }, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + s := &ImageConfigStore{ + client: tc.args.client, + } + got, err := s.bestMatch(context.Background(), tc.args.image, tc.args.isValid) + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("bestMatch() error -want +got: %s", diff) + } + if diff := cmp.Diff(tc.want.config, got, cmp.AllowUnexported(v1beta1.ImageConfig{})); diff != "" { + t.Errorf("bestMatch() config -want +got: %s", diff) + } + }) + } +} diff --git a/internal/xpkg/fake/config.go b/internal/xpkg/fake/config.go new file mode 100644 index 000000000..c94bcbbe8 --- /dev/null +++ b/internal/xpkg/fake/config.go @@ -0,0 +1,26 @@ +package fake + +import ( + "context" + + "github.com/crossplane/crossplane/internal/xpkg" +) + +var _ xpkg.ConfigStore = &MockConfigStore{} + +// MockConfigStore is a mock ConfigStore. +type MockConfigStore struct { + MockPullSecretFor func(ctx context.Context, image string) (imageConfig string, pullSecret string, err error) +} + +// PullSecretFor calls the underlying MockPullSecretFor. +func (s *MockConfigStore) PullSecretFor(ctx context.Context, image string) (imageConfig string, pullSecret string, err error) { + return s.MockPullSecretFor(ctx, image) +} + +// NewMockConfigStorePullSecretForFn creates a new MockPullSecretFor function for MockConfigStore. +func NewMockConfigStorePullSecretForFn(imageConfig, pullSecret string, err error) func(context.Context, string) (string, string, error) { + return func(context.Context, string) (string, string, error) { + return imageConfig, pullSecret, err + } +} diff --git a/internal/xpkg/lint.go b/internal/xpkg/lint.go index 40860334b..fa9ed6591 100644 --- a/internal/xpkg/lint.go +++ b/internal/xpkg/lint.go @@ -28,7 +28,6 @@ import ( v1 "github.com/crossplane/crossplane/apis/apiextensions/v1" pkgmetav1 "github.com/crossplane/crossplane/apis/pkg/meta/v1" - pkgmetav1beta1 "github.com/crossplane/crossplane/apis/pkg/meta/v1beta1" "github.com/crossplane/crossplane/internal/version" ) @@ -98,7 +97,8 @@ func IsConfiguration(o runtime.Object) error { // IsFunction checks that an object is a Function meta type. func IsFunction(o runtime.Object) error { - if _, ok := o.(*pkgmetav1beta1.Function); !ok { + po, _ := TryConvert(o, &pkgmetav1.Function{}) + if _, ok := po.(*pkgmetav1.Function); !ok { return errors.New(errNotMetaFunction) } return nil diff --git a/internal/xpkg/lint_test.go b/internal/xpkg/lint_test.go index a1786ee69..8dff8d340 100644 --- a/internal/xpkg/lint_test.go +++ b/internal/xpkg/lint_test.go @@ -61,7 +61,7 @@ kind: Configuration metadata: name: test`) - v1alpha1FuncBytes = []byte(`apiVersion: meta.pkg.crossplane.io/v1 + v1beta1FuncBytes = []byte(`apiVersion: meta.pkg.crossplane.io/v1beta1 kind: Function metadata: name: test`) @@ -73,6 +73,11 @@ metadata: v1ConfBytes = []byte(`apiVersion: meta.pkg.crossplane.io/v1 kind: Configuration +metadata: + name: test`) + + v1FuncBytes = []byte(`apiVersion: meta.pkg.crossplane.io/v1 +kind: Function metadata: name: test`) @@ -94,12 +99,14 @@ metadata: _ = yaml.Unmarshal(v1alpha1ProvBytes, v1alpha1ProvMeta) v1alpha1ConfMeta = &pkgmetav1alpha1.Configuration{} _ = yaml.Unmarshal(v1alpha1ConfBytes, v1alpha1ConfMeta) - v1alpha1FuncMeta = &pkgmetav1beta1.Function{} - _ = yaml.Unmarshal(v1alpha1FuncBytes, v1alpha1FuncMeta) + v1beta1FuncMeta = &pkgmetav1beta1.Function{} + _ = yaml.Unmarshal(v1beta1FuncBytes, v1beta1FuncMeta) v1ProvMeta = &pkgmetav1.Provider{} _ = yaml.Unmarshal(v1ProvBytes, v1ProvMeta) v1ConfMeta = &pkgmetav1.Configuration{} _ = yaml.Unmarshal(v1ConfBytes, v1ConfMeta) + v1FuncMeta = &pkgmetav1.Function{} + _ = yaml.Unmarshal(v1FuncBytes, v1FuncMeta) v1XRD = &v1.CompositeResourceDefinition{} _ = yaml.Unmarshal(v1XRDBytes, v1XRD) v1Comp = &v1.Composition{} @@ -220,9 +227,15 @@ func TestIsFunction(t *testing.T) { obj runtime.Object err error }{ - "v1alpha1": { - reason: "Should not return error if object is a v1alpha1 function.", - obj: v1alpha1FuncMeta, + // Function packages were introduced at v1beta1. There was never a + // v1alpha1 version of the package metadata. + "v1beta1": { + reason: "Should not return error if object is a v1beta1 function.", + obj: v1beta1FuncMeta, + }, + "v1": { + reason: "Should not return error if object is a v1 function.", + obj: v1FuncMeta, }, "ErrNotFunction": { reason: "Should return error if object is not function.", diff --git a/test/e2e/manifests/apiextensions/composition/functions/setup/functions.yaml b/test/e2e/manifests/apiextensions/composition/functions/setup/functions.yaml index 3aa7ac1f6..caadf924c 100644 --- a/test/e2e/manifests/apiextensions/composition/functions/setup/functions.yaml +++ b/test/e2e/manifests/apiextensions/composition/functions/setup/functions.yaml @@ -1,4 +1,5 @@ --- +# We intentionally use v1beta1 here, to make sure it still works. apiVersion: pkg.crossplane.io/v1beta1 kind: Function metadata: @@ -6,12 +7,18 @@ metadata: spec: # NOTE(negz): This is currently manually pushed. See README.md at # https://github.com/crossplane-contrib/function-dummy. + # NOTE(negz): This version of the function uses meta.pkg.crossplane.io/v1beta1 + # and is built with an old SDK that only serves v1beta1 RPCs. This is + # intentional. We want to make sure Crossplane is backward compatible with + # older v1beta1 functions. package: xpkg.upbound.io/crossplane-contrib/function-dummy:v0.2.1 --- -apiVersion: pkg.crossplane.io/v1beta1 +apiVersion: pkg.crossplane.io/v1 kind: Function metadata: name: function-auto-ready spec: - # NOTE(negz): This is also currently manually pushed. - package: xpkg.upbound.io/crossplane-contrib/function-auto-ready:v0.2.1 + # TODO(negz): This function should use meta.pkg.crossplane.io/v1 metadata. + # It supports the new v1 RPCs but it can't be built using v1 metadata until + # https://github.com/crossplane/crossplane/issues/5971 is fixed. + package: xpkg.upbound.io/crossplane-contrib/function-auto-ready:v0.3.0 diff --git a/test/e2e/manifests/pkg/configuration/digest/configuration-revision.yaml b/test/e2e/manifests/pkg/configuration/digest/configuration-revision.yaml new file mode 100644 index 000000000..f645e5bf0 --- /dev/null +++ b/test/e2e/manifests/pkg/configuration/digest/configuration-revision.yaml @@ -0,0 +1,7 @@ +# We don't install this directly, but we do use it to ensure the configuration is completely deleted. +# Please note, its name will change if configuration package contents changes. +apiVersion: pkg.crossplane.io/v1 +kind: Configuration +metadata: + name: configuration-digest-127afe7f55ed +spec: {} diff --git a/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/configuration-revision.yaml b/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/configuration-revision.yaml new file mode 100644 index 000000000..e706f7b58 --- /dev/null +++ b/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/configuration-revision.yaml @@ -0,0 +1,7 @@ +# We don't install this directly, but we do use it to ensure the configuration is completely deleted. +# Please note, its name will change if configuration package contents changes. +apiVersion: pkg.crossplane.io/v1 +kind: ConfigurationRevision +metadata: + name: e2e-configuration-with-private-dependency-e5b6aa4500c3 +spec: {} \ No newline at end of file diff --git a/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/configuration.yaml b/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/configuration.yaml new file mode 100644 index 000000000..e70b9438e --- /dev/null +++ b/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/configuration.yaml @@ -0,0 +1,7 @@ +# This configuration has a private dependency. See package/crossplane.yaml. +apiVersion: pkg.crossplane.io/v1 +kind: Configuration +metadata: + name: e2e-configuration-with-private-dependency +spec: + package: xpkg.upbound.io/crossplane/e2e-configuration-with-private-dependency:v0.1.0 \ No newline at end of file diff --git a/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/image-config.yaml b/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/image-config.yaml new file mode 100644 index 000000000..0e2959c00 --- /dev/null +++ b/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/image-config.yaml @@ -0,0 +1,11 @@ +apiVersion: pkg.crossplane.io/v1beta1 +kind: ImageConfig +metadata: + name: e2e-private-provider-auth +spec: + matchImages: + - prefix: xpkg.upbound.io/crossplane/e2e-private-provider + registry: + authentication: + pullSecretRef: + name: e2e-private-provider-pull-secret \ No newline at end of file diff --git a/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/package/crossplane.yaml b/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/package/crossplane.yaml new file mode 100644 index 000000000..8d8bd166a --- /dev/null +++ b/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/package/crossplane.yaml @@ -0,0 +1,8 @@ +apiVersion: meta.pkg.crossplane.io/v1alpha1 +kind: Configuration +metadata: + name: e2e-configuration-with-private-dependency +spec: + dependsOn: + - provider: xpkg.upbound.io/crossplane/e2e-private-provider + version: ">=v0.0.1" diff --git a/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/provider.yaml b/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/provider.yaml new file mode 100644 index 000000000..08214d261 --- /dev/null +++ b/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/provider.yaml @@ -0,0 +1,8 @@ +# We don't install this directly, but we do install the configuration that depends on it. +# We will only use this manifest to verify that the private dependency is installed. +apiVersion: pkg.crossplane.io/v1 +kind: Provider +metadata: + name: crossplane-e2e-private-provider +spec: + package: xpkg.upbound.io/crossplane/e2e-private-provider:v0.1.0 \ No newline at end of file diff --git a/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/pull-secret.yaml b/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/pull-secret.yaml new file mode 100644 index 000000000..7e9b2731d --- /dev/null +++ b/test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency/pull-secret.yaml @@ -0,0 +1,10 @@ +# Checking in the credentials in plaintext is intentional as the token only has permission to pull the private image +# used in the e2e tests, i.e. "xpkg.upbound.io/crossplane/e2e-private-provider". +apiVersion: v1 +kind: Secret +metadata: + namespace: crossplane-system + name: e2e-private-provider-pull-secret +type: kubernetes.io/dockerconfigjson +data: + .dockerconfigjson: eyJhdXRocyI6eyJ4cGtnLnVwYm91bmQuaW8iOnsidXNlcm5hbWUiOiI4YzhlMGQyMi01YTgzLTQyMTctODk4NS1kOGVlNTNkOTFiMGIiLCJwYXNzd29yZCI6ImV5SmhiR2NpT2lKU1V6STFOaUlzSW10cFpDSTZJbFJmSWl3aWRIbHdJam9pU2xkVUluMC5leUpoZFdRaU9pSjFjR0p2ZFc1a0xtbHZJaXdpWlhod0lqb3lNRFF6T1RBd016RTBMQ0pxZEdraU9pSTRZemhsTUdReU1pMDFZVGd6TFRReU1UY3RPRGs0TlMxa09HVmxOVE5rT1RGaU1HSWlMQ0pwYzNNaU9pSm9kSFJ3Y3pvdkwyRndhUzUxY0dKdmRXNWtMbWx2TDNZeElpd2ljM1ZpSWpvaWNtOWliM1I4T0RrelpqVmxaamt0T1RJMFpTMDBZalE1TFdJMU1tWXRZek13TkdRNVpqQTVNR00xSW4wLlQzNjBBclpEMjJzNU5FV3M2cEdMU3kxRHVKc190RFVsTzZNNHpTYnF4b292QVgtWDNEb3FUc0ZBd20tVTg3VWJaZVBDX21FWGl6R1NYQTJ5MkNtaVB2RXpWdGNEMl9EbEJkd18weTVVRGNfWjhBdVhETWdpQThYSXNPSjZaSEZNVE0walY1bF9jQ25qTEx4ZUw0Q0hCcF9seTN3Z3dDVEFXT2tNN3N5WG9LUG9vY0wtRFlVei1NR2s3ZlVFTGlmOUFXYnd4WndHVTFVVnVyTFlJMzBvYXdiWnFDV3BTTmhPd0hUZFJobF9ZTmFsRnhQUVI3clUtQjNwVGdsQ1ZyVExYelhaM1hzZFZhUTU5UjdFUmNaZkRqLUdFY0RxNkRVS29VQnJPbEZvdjM0bFo0a1FzbjRTZEtLTFpzRDF2eEw3R0hhR2djX21NSHBIWGR3dHd0WklXdyIsImF1dGgiOiJPR000WlRCa01qSXROV0U0TXkwME1qRTNMVGc1T0RVdFpEaGxaVFV6WkRreFlqQmlPbVY1U21oaVIyTnBUMmxLVTFWNlNURk9hVWx6U1cxMGNGcERTVFpKYkZKbVNXbDNhV1JJYkhkSmFtOXBVMnhrVlVsdU1DNWxlVXBvWkZkUmFVOXBTakZqUjBwMlpGYzFhMHh0YkhaSmFYZHBXbGhvZDBscWIzbE5SRkY2VDFSQmQwMTZSVEJNUTBweFpFZHJhVTlwU1RSWmVtaHNUVWRSZVUxcE1ERlpWR2Q2VEZSUmVVMVVZM1JQUkdzMFRsTXhhMDlIVm14T1ZFNXJUMVJHYVUxSFNXbE1RMHB3WXpOTmFVOXBTbTlrU0ZKM1kzcHZka3d5Um5kaFV6VXhZMGRLZG1SWE5XdE1iV3gyVEROWmVFbHBkMmxqTTFacFNXcHZhV050T1dsaU0xSTRUMFJyZWxwcVZteGFhbXQwVDFSSk1GcFRNREJaYWxFMVRGZEpNVTF0V1hSWmVrMTNUa2RSTlZwcVFUVk5SMDB4U1c0d0xsUXpOakJCY2xwRU1qSnpOVTVGVjNNMmNFZE1VM2t4UkhWS2MxOTBSRlZzVHpaTk5IcFRZbkY0YjI5MlFWZ3RXRE5FYjNGVWMwWkJkMjB0VlRnM1ZXSmFaVkJEWDIxRldHbDZSMU5ZUVRKNU1rTnRhVkIyUlhwV2RHTkVNbDlFYkVKa2QxOHdlVFZWUkdOZldqaEJkVmhFVFdkcFFUaFlTWE5QU2paYVNFWk5WRTB3YWxZMWJGOWpRMjVxVEV4NFpVdzBRMGhDY0Y5c2VUTjNaM2REVkVGWFQydE5OM041V0c5TFVHOXZZMHd0UkZsVmVpMU5SMnMzWmxWRlRHbG1PVUZYWW5kNFduZEhWVEZWVm5WeVRGbEpNekJ2WVhkaVduRkRWM0JUVG1oUGQwaFVaRkpvYkY5WlRtRnNSbmhRVVZJM2NsVXRRak53Vkdkc1ExWnlWRXhZZWxoYU0xaHpaRlpoVVRVNVVqZEZVbU5hWmtScUxVZEZZMFJ4TmtSVlMyOVZRbkpQYkVadmRqTTBiRm8wYTFGemJqUlRaRXRMVEZwelJERjJlRXczUjBoaFIyZGpYMjFOU0hCSVdHUjNkSGQwV2tsWGR3PT0ifX19 \ No newline at end of file diff --git a/test/e2e/pkg_test.go b/test/e2e/pkg_test.go index 9355b0a72..01329f410 100644 --- a/test/e2e/pkg_test.go +++ b/test/e2e/pkg_test.go @@ -230,3 +230,39 @@ func TestExternallyManagedServiceAccount(t *testing.T) { Feature(), ) } + +func TestImageConfigAuth(t *testing.T) { + manifests := "test/e2e/manifests/pkg/image-config/authentication/configuration-with-private-dependency" + + environment.Test(t, + features.NewWithDescription(t.Name(), "Tests that we can install a private package as a dependency by providing registry pull credentials through ImageConfig API."). + WithLabel(LabelArea, LabelAreaPkg). + WithLabel(LabelSize, LabelSizeSmall). + WithLabel(config.LabelTestSuite, config.TestSuiteDefault). + WithSetup("ApplyImageConfig", funcs.AllOf( + funcs.ApplyResources(FieldManager, manifests, "pull-secret.yaml"), + funcs.ApplyResources(FieldManager, manifests, "image-config.yaml"), + funcs.ApplyResources(FieldManager, manifests, "configuration.yaml"), + funcs.ResourcesCreatedWithin(1*time.Minute, manifests, "configuration.yaml"), + )). + Assess("ProviderInstalledAndHealthy", funcs.AllOf( + funcs.ResourcesHaveConditionWithin(2*time.Minute, manifests, "provider.yaml", pkgv1.Healthy(), pkgv1.Active()), + )). + Assess("ConfigurationInstalledAndHealthy", funcs.AllOf( + funcs.ResourcesHaveConditionWithin(2*time.Minute, manifests, "provider.yaml", pkgv1.Healthy(), pkgv1.Active()), + )). + WithTeardown("DeleteConfiguration", funcs.AllOf( + funcs.DeleteResources(manifests, "configuration.yaml"), + funcs.ResourcesDeletedWithin(1*time.Minute, manifests, "configuration.yaml"), + // We wait until the configuration revision is gone, otherwise + // the provider we will be deleting next might come back as a + // result of the configuration revision being reconciled again. + funcs.ResourcesDeletedWithin(1*time.Minute, manifests, "configuration-revision.yaml"), + )). + // Dependencies are not automatically deleted. + WithTeardown("DeleteProvider", funcs.AllOf( + funcs.DeleteResources(manifests, "provider.yaml"), + funcs.ResourcesDeletedWithin(1*time.Minute, manifests, "provider.yaml"), + )).Feature(), + ) +}