diff --git a/Earthfile b/Earthfile index 1eec76e68..ec78e105c 100644 --- a/Earthfile +++ b/Earthfile @@ -162,9 +162,17 @@ go-build: CACHE --id go-build --sharing shared /root/.cache/go-build COPY --dir apis/ cmd/ internal/ pkg/ . RUN go build -o crossplane${ext} ./cmd/crossplane + RUN sha256sum crossplane${ext} | head -c 64 > crossplane${ext}.sha256 RUN go build -o crank${ext} ./cmd/crank - SAVE ARTIFACT crossplane${ext} AS LOCAL _output/bin/${GOOS}_${GOARCH}/crossplane${ext} - SAVE ARTIFACT crank${ext} AS LOCAL _output/bin/${GOOS}_${GOARCH}/crank${ext} + RUN sha256sum crank${ext} | head -c 64 > crank${ext}.sha256 + RUN tar -czvf crank.tar.gz crank${ext} crank${ext}.sha256 + RUN sha256sum crank.tar.gz | head -c 64 > crank.tar.gz.sha256 + SAVE ARTIFACT --keep-ts crossplane${ext} AS LOCAL _output/bin/${GOOS}_${GOARCH}/crossplane${ext} + SAVE ARTIFACT --keep-ts crossplane${ext}.sha256 AS LOCAL _output/bin/${GOOS}_${GOARCH}/crossplane${ext}.sha256 + SAVE ARTIFACT --keep-ts crank${ext} AS LOCAL _output/bin/${GOOS}_${GOARCH}/crank${ext} + SAVE ARTIFACT --keep-ts crank${ext}.sha256 AS LOCAL _output/bin/${GOOS}_${GOARCH}/crank${ext}.sha256 + SAVE ARTIFACT --keep-ts crank.tar.gz AS LOCAL _output/bundle/${GOOS}_${GOARCH}/crank.tar.gz + SAVE ARTIFACT --keep-ts crank.tar.gz.sha256 AS LOCAL _output/bundle/${GOOS}_${GOARCH}/crank.tar.gz.sha256 # go-multiplatform-build builds Crossplane binaries for all supported OS # and architectures. diff --git a/apis/pkg/v1beta1/lock.go b/apis/pkg/v1beta1/lock.go index c6c102146..9a8ce0a8e 100644 --- a/apis/pkg/v1beta1/lock.go +++ b/apis/pkg/v1beta1/lock.go @@ -94,7 +94,7 @@ type Dependency struct { // Type is the type of package. Can be either Configuration or Provider. Type PackageType `json:"type"` - // Constraints is a valid semver range, which will be used to select a valid + // Constraints is a valid semver range or a digest, which will be used to select a valid // dependency version. Constraints string `json:"constraints"` } diff --git a/cluster/crds/pkg.crossplane.io_locks.yaml b/cluster/crds/pkg.crossplane.io_locks.yaml index b55f731bf..50f4f393b 100644 --- a/cluster/crds/pkg.crossplane.io_locks.yaml +++ b/cluster/crds/pkg.crossplane.io_locks.yaml @@ -54,7 +54,7 @@ spec: properties: constraints: description: |- - Constraints is a valid semver range, which will be used to select a valid + Constraints is a valid semver range or a digest, which will be used to select a valid dependency version. type: string package: diff --git a/cmd/crank/render/cmd.go b/cmd/crank/render/cmd.go index ee50633bc..f0bfbabb1 100644 --- a/cmd/crank/render/cmd.go +++ b/cmd/crank/render/cmd.go @@ -25,6 +25,7 @@ import ( "github.com/alecthomas/kong" "github.com/spf13/afero" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/serializer/json" @@ -48,9 +49,10 @@ type Cmd struct { ContextValues map[string]string `help:"Comma-separated context key-value pairs to pass to the Function pipeline. Values must be JSON. Keys take precedence over --context-files." mapsep:""` IncludeFunctionResults bool `help:"Include informational and warning messages from Functions in the rendered output as resources of kind: Result." short:"r"` IncludeFullXR bool `help:"Include a direct copy of the input XR's spec and metadata fields in the rendered output." short:"x"` - ObservedResources string `help:"A YAML file or directory of YAML files specifying the observed state of composed resources." placeholder:"PATH" short:"o" type:"path"` - ExtraResources string `help:"A YAML file or directory of YAML files specifying extra resources to pass to the Function pipeline." placeholder:"PATH" short:"e" type:"path"` + ObservedResources string `help:"A YAML file or directory of YAML files specifying the observed state of composed resources." placeholder:"PATH" short:"o" type:"path"` + ExtraResources string `help:"A YAML file or directory of YAML files specifying extra resources to pass to the Function pipeline." placeholder:"PATH" short:"e" type:"path"` IncludeContext bool `help:"Include the context in the rendered output as a resource of kind: Context." short:"c"` + FunctionCredentials string `help:"A YAML file or directory of YAML files specifying credentials to use for Functions to render the XR." placeholder:"PATH" type:"path"` Timeout time.Duration `default:"1m" help:"How long to run before timing out."` @@ -109,6 +111,10 @@ Examples: # Pass extra resources Functions in the pipeline can request. crossplane render xr.yaml composition.yaml functions.yaml \ --extra-resources=extra-resources.yaml + + # Pass credentials to Functions in the pipeline that need them. + crossplane render xr.yaml composition.yaml functions.yaml \ + --function-credentials=credentials.yaml ` } @@ -149,6 +155,14 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger) error { //nolint:gocognit return errors.Wrapf(err, "cannot load functions from %q", c.Functions) } + fcreds := []corev1.Secret{} + if c.FunctionCredentials != "" { + fcreds, err = LoadCredentials(c.fs, c.FunctionCredentials) + if err != nil { + return errors.Wrapf(err, "cannot load secrets from %q", c.FunctionCredentials) + } + } + ors := []composed.Unstructured{} if c.ObservedResources != "" { ors, err = LoadObservedResources(c.fs, c.ObservedResources) @@ -181,12 +195,13 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger) error { //nolint:gocognit defer cancel() out, err := Render(ctx, log, Inputs{ - CompositeResource: xr, - Composition: comp, - Functions: fns, - ObservedResources: ors, - ExtraResources: ers, - Context: fctx, + CompositeResource: xr, + Composition: comp, + Functions: fns, + FunctionCredentials: fcreds, + ObservedResources: ors, + ExtraResources: ers, + Context: fctx, }) if err != nil { return errors.Wrap(err, "cannot render composite resource") diff --git a/cmd/crank/render/load.go b/cmd/crank/render/load.go index 6a1d11765..b0081aef9 100644 --- a/cmd/crank/render/load.go +++ b/cmd/crank/render/load.go @@ -22,6 +22,7 @@ import ( "path/filepath" "github.com/spf13/afero" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/yaml" @@ -94,6 +95,25 @@ func LoadFunctions(filesys afero.Fs, file string) ([]pkgv1.Function, error) { return functions, nil } +// LoadCredentials from a stream of YAML manifests. +func LoadCredentials(fs afero.Fs, file string) ([]corev1.Secret, error) { + stream, err := LoadYAMLStream(fs, file) + if err != nil { + return nil, errors.Wrap(err, "cannot load YAML stream from file") + } + + secrets := make([]corev1.Secret, 0, len(stream)) + for _, y := range stream { + s := &corev1.Secret{} + if err := yaml.Unmarshal(y, s); err != nil { + return nil, errors.Wrap(err, "cannot parse YAML secret manifest") + } + secrets = append(secrets, *s) + } + + return secrets, nil +} + // LoadExtraResources from a stream of YAML manifests. func LoadExtraResources(fs afero.Fs, file string) ([]unstructured.Unstructured, error) { stream, err := LoadYAMLStream(fs, file) diff --git a/cmd/crank/render/render.go b/cmd/crank/render/render.go index 9e8accda3..a96424e9d 100644 --- a/cmd/crank/render/render.go +++ b/cmd/crank/render/render.go @@ -27,6 +27,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" "google.golang.org/protobuf/types/known/structpb" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" @@ -66,12 +67,13 @@ const ( // Inputs contains all inputs to the render process. type Inputs struct { - CompositeResource *ucomposite.Unstructured - Composition *apiextensionsv1.Composition - Functions []pkgv1.Function - ObservedResources []composed.Unstructured - ExtraResources []unstructured.Unstructured - Context map[string][]byte + CompositeResource *ucomposite.Unstructured + Composition *apiextensionsv1.Composition + Functions []pkgv1.Function + FunctionCredentials []corev1.Secret + ObservedResources []composed.Unstructured + ExtraResources []unstructured.Unstructured + Context map[string][]byte // TODO(negz): Allow supplying observed XR and composed resource connection // details. Maybe as Secrets? What if secret stores are in use? @@ -159,6 +161,16 @@ func (r *RuntimeFunctionRunner) Stop(ctx context.Context) error { return nil } +// getSecret retrieves the secret with the specified name and namespace from the provided list of secrets. +func getSecret(name string, nameSpace string, secrets []corev1.Secret) (*corev1.Secret, error) { + for _, s := range secrets { + if s.GetName() == name && s.GetNamespace() == nameSpace { + return &s, nil + } + } + return nil, errors.Errorf("secret %q not found", name) +} + // Render the desired XR and composed resources, sorted by resource name, given the supplied inputs. func Render(ctx context.Context, log logging.Logger, in Inputs) (Outputs, error) { //nolint:gocognit // TODO(negz): Should we refactor to break this up a bit? runtimes, err := NewRuntimeFunctionRunner(ctx, log, in.Functions) @@ -227,6 +239,26 @@ func Render(ctx context.Context, log logging.Logger, in Inputs) (Outputs, error) req.Input = in } + req.Credentials = map[string]*fnv1.Credentials{} + for _, cs := range fn.Credentials { + // For now we only support loading credentials from secrets. + if cs.Source != apiextensionsv1.FunctionCredentialsSourceSecret || cs.SecretRef == nil { + continue + } + + s, err := getSecret(cs.SecretRef.Name, cs.SecretRef.Namespace, in.FunctionCredentials) + if err != nil { + return Outputs{}, errors.Wrapf(err, "cannot get credentials from secret %q", cs.SecretRef.Name) + } + req.Credentials[cs.Name] = &fnv1.Credentials{ + Source: &fnv1.Credentials_CredentialData{ + CredentialData: &fnv1.CredentialData{ + Data: s.Data, + }, + }, + } + } + rsp, err := runner.RunFunction(ctx, fn.FunctionRef.Name, req) if err != nil { return Outputs{}, errors.Wrapf(err, "cannot run pipeline step %q", fn.Step) diff --git a/cmd/crank/render/render_test.go b/cmd/crank/render/render_test.go index 4f0600697..b9308bf83 100644 --- a/cmd/crank/render/render_test.go +++ b/cmd/crank/render/render_test.go @@ -28,6 +28,7 @@ import ( "google.golang.org/grpc/credentials/insecure" "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/types/known/structpb" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -1046,6 +1047,58 @@ func TestFilterExtraResources(t *testing.T) { } } +func TestGetSecret(t *testing.T) { + secrets := []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "secret1", + Namespace: "namespace1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "secret2", + Namespace: "namespace2", + }, + }, + } + + tests := map[string]struct { + name string + namespace string + secrets []corev1.Secret + wantErr bool + }{ + "SecretFound": { + name: "secret1", + namespace: "namespace1", + secrets: secrets, + wantErr: false, + }, + "SecretNotFound": { + name: "secret3", + namespace: "namespace3", + secrets: secrets, + wantErr: true, + }, + "SecretWrongNamespace": { + name: "secret1", + namespace: "namespace2", + secrets: secrets, + wantErr: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + _, err := getSecret(tc.name, tc.namespace, tc.secrets) + if (err != nil) != tc.wantErr { + t.Errorf("getSecret() error = %v, wantErr %v", err, tc.wantErr) + } + }) + } +} + func MustStructJSON(j string) *structpb.Struct { s := &structpb.Struct{} if err := protojson.Unmarshal([]byte(j), s); err != nil { diff --git a/internal/controller/pkg/manager/revisioner_test.go b/internal/controller/pkg/manager/revisioner_test.go index 93807eea2..668b76563 100644 --- a/internal/controller/pkg/manager/revisioner_test.go +++ b/internal/controller/pkg/manager/revisioner_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + conregv1 "github.com/google/go-containerregistry/pkg/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -96,6 +97,33 @@ func TestPackageRevisioner(t *testing.T) { digest: "return-me", }, }, + "SuccessfulDigest": { + reason: "Should return the digest of the package source image.", + args: args{ + pkg: &v1.Provider{ + ObjectMeta: metav1.ObjectMeta{ + Name: "provider-nop", + }, + Spec: v1.ProviderSpec{ + PackageSpec: v1.PackageSpec{ + Package: "crossplane-contrib/provider-nop@sha256:ecc25c121431dfc7058754427f97c034ecde26d4aafa0da16d258090e0443904", + PackagePullPolicy: &pullIfNotPresent, + }, + }, + }, + f: &fake.MockFetcher{ + MockHead: fake.NewMockHeadFn(&conregv1.Descriptor{ + Digest: conregv1.Hash{ + Algorithm: "sha256", + Hex: "ecc25c121431dfc7058754427f97c034ecde26d4aafa0da16d258090e0443904", + }, + }, nil), + }, + }, + want: want{ + digest: "provider-nop-ecc25c121431", + }, + }, "ErrParseRef": { reason: "Should return an error if we cannot parse reference from package source image.", args: args{ diff --git a/internal/controller/pkg/resolver/reconciler.go b/internal/controller/pkg/resolver/reconciler.go index c0bc15f9d..646fbee0f 100644 --- a/internal/controller/pkg/resolver/reconciler.go +++ b/internal/controller/pkg/resolver/reconciler.go @@ -26,6 +26,7 @@ import ( "github.com/Masterminds/semver" "github.com/google/go-containerregistry/pkg/name" + conregv1 "github.com/google/go-containerregistry/pkg/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/client-go/kubernetes" ctrl "sigs.k8s.io/controller-runtime" @@ -48,7 +49,8 @@ import ( const ( reconcileTimeout = 1 * time.Minute - packageTagFmt = "%s:%s" + packageTagFmt = "%s:%s" + packageDigestFmt = "%s@%s" ) const ( @@ -235,42 +237,16 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco log.Debug(errInvalidDependency, "error", errors.Errorf(errFmtMissingDependency, dep.Identifier())) return reconcile.Result{Requeue: false}, nil } - c, err := semver.NewConstraint(dep.Constraints) - if err != nil { - log.Debug(errInvalidConstraint, "error", err) - return reconcile.Result{Requeue: false}, nil - } + ref, err := name.ParseReference(dep.Package, name.WithDefaultRegistry(r.registry)) if err != nil { log.Debug(errInvalidDependency, "error", err) return reconcile.Result{Requeue: false}, nil } - // 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) - if err != nil { - log.Debug(errFetchTags, "error", err) - return reconcile.Result{}, errors.Wrap(err, errFetchTags) - } - - vs := []*semver.Version{} - for _, r := range tags { - v, err := semver.NewVersion(r) - if err != nil { - // We skip any tags that are not valid semantic versions. - continue - } - vs = append(vs, v) - } - - sort.Sort(semver.Collection(vs)) var addVer string - for _, v := range vs { - if c.Check(v) { - addVer = v.Original() - } + if addVer, err = r.findDependencyVersion(ctx, dep, log, ref); err != nil { + return reconcile.Result{Requeue: false}, errors.New(errInvalidDependency) } // NOTE(hasheddan): consider creating event on package revision @@ -298,14 +274,63 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco // no packagePullSecrets are set. Settings can be modified manually // after dependency creation to address this. pack.SetName(xpkg.ToDNSLabel(ref.Context().RepositoryStr())) - pack.SetSource(fmt.Sprintf(packageTagFmt, ref.String(), addVer)) + + format := packageTagFmt + if strings.HasPrefix(addVer, "sha256:") { + format = packageDigestFmt + } + + pack.SetSource(fmt.Sprintf(format, ref.String(), addVer)) // NOTE(hasheddan): consider making the lock the controller of packages // it creates. - if err := r.client.Create(ctx, pack); err != nil { + if err := r.client.Create(ctx, pack); err != nil && !kerrors.IsAlreadyExists(err) { log.Debug(errCreateDependency, "error", err) return reconcile.Result{}, errors.Wrap(err, errCreateDependency) } return reconcile.Result{Requeue: false}, nil } + +func (r *Reconciler) findDependencyVersion(ctx context.Context, dep *v1beta1.Dependency, log logging.Logger, ref name.Reference) (string, error) { + var addVer string + + if digest, err := conregv1.NewHash(dep.Constraints); err == nil { + log.Debug("package is pinned to a specific digest, skipping resolution") + return digest.String(), nil + } + + c, err := semver.NewConstraint(dep.Constraints) + if err != nil { + log.Debug(errInvalidConstraint, "error", err) + return "", errors.New(errInvalidConstraint) + } + + // 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) + if err != nil { + log.Debug(errFetchTags, "error", err) + return "", errors.New(errFetchTags) + } + + vs := []*semver.Version{} + for _, r := range tags { + v, err := semver.NewVersion(r) + if err != nil { + // We skip any tags that are not valid semantic versions. + continue + } + vs = append(vs, v) + } + + sort.Sort(semver.Collection(vs)) + for _, v := range vs { + if c.Check(v) { + addVer = v.Original() + } + } + + return addVer, nil +} diff --git a/internal/controller/pkg/resolver/reconciler_test.go b/internal/controller/pkg/resolver/reconciler_test.go index b8cfa7efd..78efa1f99 100644 --- a/internal/controller/pkg/resolver/reconciler_test.go +++ b/internal/controller/pkg/resolver/reconciler_test.go @@ -254,6 +254,45 @@ func TestReconcile(t *testing.T) { r: reconcile.Result{Requeue: false}, }, }, + "SuccessfulNoMissingWithDigest": { + reason: "We should not return error and not requeue if no missing dependencies with digest.", + 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: "sha256:ecc25c121431dfc7058754427f97c034ecde26d4aafa0da16d258090e0443904", + }) + 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 nil, nil + }, + MockSort: func() ([]string, error) { + return nil, nil + }, + } + }), + }, + }, + want: want{ + r: reconcile.Result{Requeue: false}, + }, + }, "ErrorInvalidDependency": { reason: "We should not requeue if dependency is invalid.", args: args{ @@ -294,7 +333,8 @@ func TestReconcile(t *testing.T) { }, }, want: want{ - r: reconcile.Result{Requeue: false}, + r: reconcile.Result{Requeue: false}, + err: errors.New(errInvalidDependency), }, }, "ErrorFetchTags": { @@ -341,7 +381,7 @@ func TestReconcile(t *testing.T) { }, }, want: want{ - err: errors.Wrap(errBoom, errFetchTags), + err: errors.New(errInvalidDependency), }, }, "ErrorNoValidVersion": { @@ -440,6 +480,55 @@ func TestReconcile(t *testing.T) { err: errors.Wrap(errBoom, errCreateDependency), }, }, + "ErrorCreateMissingDependencyWithDigest": { + reason: "We should return an error if unable to create missing dependency with digest.", + 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: "sha256:ecc25c121431dfc7058754427f97c034ecde26d4aafa0da16d258090e0443904", + }) + return nil + }), + MockCreate: test.NewMockCreateFn(errBoom), + 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: "hasheddan/config-nop-c", + Constraints: "sha256:ecc25c121431dfc7058754427f97c034ecde26d4aafa0da16d258090e0443904", + Type: v1beta1.ConfigurationPackageType, + }, + }, nil + }, + MockSort: func() ([]string, error) { + return nil, nil + }, + } + }), + WithFetcher(&fakexpkg.MockFetcher{ + MockTags: fakexpkg.NewMockTagsFn([]string{"v0.2.0", "v0.3.0", "v1.0.0", "v1.2.0"}, nil), + }), + }, + }, + want: want{ + err: errors.Wrap(errBoom, errCreateDependency), + }, + }, "SuccessfulCreateMissingDependency": { reason: "We should not requeue if able to create missing dependency.", args: args{ @@ -489,6 +578,52 @@ func TestReconcile(t *testing.T) { r: reconcile.Result{Requeue: false}, }, }, + "SuccessfulCreateMissingDependencyWithDigest": { + reason: "We should not requeue if able to create missing dependency with digest.", + 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 + }), + MockCreate: test.NewMockCreateFn(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: "hasheddan/provider-nop-c", + Constraints: "sha256:ecc25c121431dfc7058754427f97c034ecde26d4aafa0da16d258090e0443904", + Type: v1beta1.ProviderPackageType, + }, + }, nil + }, + MockSort: func() ([]string, error) { + return nil, nil + }, + } + }), + }, + }, + want: want{ + r: reconcile.Result{Requeue: false}, + }, + }, } for name, tc := range cases { diff --git a/internal/controller/pkg/revision/dependency.go b/internal/controller/pkg/revision/dependency.go index af7b76f0b..2cd8fa82e 100644 --- a/internal/controller/pkg/revision/dependency.go +++ b/internal/controller/pkg/revision/dependency.go @@ -23,6 +23,7 @@ import ( "github.com/Masterminds/semver" "github.com/google/go-containerregistry/pkg/name" + conregv1 "github.com/google/go-containerregistry/pkg/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -216,6 +217,12 @@ func (m *PackageDependencyManager) Resolve(ctx context.Context, pkg runtime.Obje if !ok { return found, installed, invalid, errors.New(errDependencyNotLockPackage) } + + // Check if the constraint is a digest, and if so, skip constraint check. + if _, err := conregv1.NewHash(dep.Constraints); err == nil { + continue + } + c, err := semver.NewConstraint(dep.Constraints) if err != nil { return found, installed, invalid, 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/dependency/provider-revision-dependency.yaml b/test/e2e/manifests/pkg/configuration/dependency/provider-revision-dependency.yaml new file mode 100644 index 000000000..fcf5c04ee --- /dev/null +++ b/test/e2e/manifests/pkg/configuration/dependency/provider-revision-dependency.yaml @@ -0,0 +1,6 @@ +# Note that we don't directly create this manifest, we just use it to check +# whether the Provider was installed. +apiVersion: pkg.crossplane.io/v1 +kind: ProviderRevision +metadata: + name: crossplane-contrib-provider-nop-552a394a8acc \ No newline at end of file diff --git a/test/e2e/manifests/pkg/configuration/digest/configuration.yaml b/test/e2e/manifests/pkg/configuration/digest/configuration.yaml new file mode 100644 index 000000000..c2c0470aa --- /dev/null +++ b/test/e2e/manifests/pkg/configuration/digest/configuration.yaml @@ -0,0 +1,7 @@ +apiVersion: pkg.crossplane.io/v1 +kind: Configuration +metadata: + name: configuration-digest +spec: + # NOTE(ezgidemirel): This package is manually built and pushed to the registry. + package: xpkg.upbound.io/crossplane/e2e-depends-on-digest@sha256:127afe7f55edf62f1c39321c39f6e74df954362d3c451ea5491cce251059d45b diff --git a/test/e2e/manifests/pkg/configuration/digest/package/crossplane.yaml b/test/e2e/manifests/pkg/configuration/digest/package/crossplane.yaml new file mode 100644 index 000000000..ce556dc1a --- /dev/null +++ b/test/e2e/manifests/pkg/configuration/digest/package/crossplane.yaml @@ -0,0 +1,13 @@ +# This is the package metadata for the Configuration installed by +# configuration.yaml. +# +# This package is manually built/pushed to +# xpkg.upbound.io/crossplane/e2e-configuration-digest +apiVersion: meta.pkg.crossplane.io/v1 +kind: Configuration +metadata: + name: configuration-digest +spec: + dependsOn: + - provider: xpkg.upbound.io/crossplane-contrib/provider-nop + version: "sha256:ecc25c121431dfc7058754427f97c034ecde26d4aafa0da16d258090e0443904" \ No newline at end of file diff --git a/test/e2e/manifests/pkg/configuration/digest/provider-dependency.yaml b/test/e2e/manifests/pkg/configuration/digest/provider-dependency.yaml new file mode 100644 index 000000000..f9b53760e --- /dev/null +++ b/test/e2e/manifests/pkg/configuration/digest/provider-dependency.yaml @@ -0,0 +1,9 @@ +# Note that we don't directly create this manifest, we just use it to check +# whether the Provider was installed. +apiVersion: pkg.crossplane.io/v1 +kind: Provider +metadata: + name: crossplane-contrib-provider-nop +spec: + package: crossplane-contrib/provider-nop@sha256:de9047e10c479de78c5007fef708d4d21feae8be10b1c748a8276d52e068fcb0 + ignoreCrossplaneConstraints: true \ No newline at end of file diff --git a/test/e2e/manifests/pkg/configuration/digest/provider-revision-dependency.yaml b/test/e2e/manifests/pkg/configuration/digest/provider-revision-dependency.yaml new file mode 100644 index 000000000..fcf5c04ee --- /dev/null +++ b/test/e2e/manifests/pkg/configuration/digest/provider-revision-dependency.yaml @@ -0,0 +1,6 @@ +# Note that we don't directly create this manifest, we just use it to check +# whether the Provider was installed. +apiVersion: pkg.crossplane.io/v1 +kind: ProviderRevision +metadata: + name: crossplane-contrib-provider-nop-552a394a8acc \ No newline at end of file diff --git a/test/e2e/pkg_test.go b/test/e2e/pkg_test.go index 34dc6daa4..4f348a084 100644 --- a/test/e2e/pkg_test.go +++ b/test/e2e/pkg_test.go @@ -69,10 +69,10 @@ func TestConfigurationWithDependency(t *testing.T) { funcs.ApplyResources(FieldManager, manifests, "configuration.yaml"), funcs.ResourcesCreatedWithin(1*time.Minute, manifests, "configuration.yaml"), )). - Assess("ConfigurationIsHealthy", - funcs.ResourcesHaveConditionWithin(2*time.Minute, manifests, "configuration.yaml", pkgv1.Healthy(), pkgv1.Active())). Assess("RequiredProviderIsHealthy", funcs.ResourcesHaveConditionWithin(2*time.Minute, manifests, "provider-dependency.yaml", pkgv1.Healthy(), pkgv1.Active())). + Assess("ConfigurationIsHealthy", + funcs.ResourcesHaveConditionWithin(2*time.Minute, manifests, "configuration.yaml", pkgv1.Healthy(), pkgv1.Active())). // Dependencies are not automatically deleted. WithTeardown("DeleteConfiguration", funcs.AllOf( funcs.DeleteResources(manifests, "configuration.yaml"), @@ -81,6 +81,9 @@ func TestConfigurationWithDependency(t *testing.T) { WithTeardown("DeleteRequiredProvider", funcs.AllOf( funcs.DeleteResources(manifests, "provider-dependency.yaml"), funcs.ResourcesDeletedWithin(1*time.Minute, manifests, "provider-dependency.yaml"), + )). + WithTeardown("DeleteProviderRevision", funcs.AllOf( + funcs.ResourcesDeletedWithin(1*time.Minute, manifests, "provider-revision-dependency.yaml"), )).Feature(), ) } @@ -224,3 +227,34 @@ func TestExternallyManagedServiceAccount(t *testing.T) { Feature(), ) } + +func TestConfigurationWithDigest(t *testing.T) { + manifests := "test/e2e/manifests/pkg/configuration/digest" + + environment.Test(t, + features.NewWithDescription(t.Name(), "Tests that a Configuration with digest which depends on a Provider with digest will become healthy when the Provider becomes healthy"). + WithLabel(LabelArea, LabelAreaPkg). + WithLabel(LabelSize, LabelSizeSmall). + WithLabel(config.LabelTestSuite, config.TestSuiteDefault). + WithSetup("ApplyConfiguration", funcs.AllOf( + funcs.ApplyResources(FieldManager, manifests, "configuration.yaml"), + funcs.ResourcesCreatedWithin(1*time.Minute, manifests, "configuration.yaml"), + )). + Assess("RequiredProviderIsHealthy", + funcs.ResourcesHaveConditionWithin(2*time.Minute, manifests, "provider-dependency.yaml", pkgv1.Healthy(), pkgv1.Active())). + Assess("ConfigurationIsHealthy", + funcs.ResourcesHaveConditionWithin(2*time.Minute, manifests, "configuration.yaml", pkgv1.Healthy(), pkgv1.Active())). + // Dependencies are not automatically deleted. + WithTeardown("DeleteConfiguration", funcs.AllOf( + funcs.DeleteResources(manifests, "configuration.yaml"), + funcs.ResourcesDeletedWithin(1*time.Minute, manifests, "configuration.yaml"), + )). + WithTeardown("DeleteRequiredProvider", funcs.AllOf( + funcs.DeleteResources(manifests, "provider-dependency.yaml"), + funcs.ResourcesDeletedWithin(1*time.Minute, manifests, "provider-dependency.yaml"), + )). + WithTeardown("DeleteProviderRevision", funcs.AllOf( + funcs.ResourcesDeletedWithin(1*time.Minute, manifests, "provider-revision-dependency.yaml"), + )).Feature(), + ) +}