Repository: crossplane/crossplane
Pull request #5669 (2024-05-09 23:47:00)

Support dependecies with version constraints in crank validate
### Description of your changes

This PR continuation of the PR #5570 where I have pointed out that the `crossplane beta validate` doesn't work with `Configuratons` that contain version constraints. This PR is trying to address that.

I have:

- [x] Read and followed Crossplane's contribution process.
- [x] Run make reviewable to ensure this PR is ready for review.
- [ ] ~~Added or updated unit tests.~~
- [ ] ~~Added or updated e2e tests.~~
- [ ] ~~Linked a PR or a docs tracking issue to document this change.~~
- [ ] ~~Added backport release-x.y labels to auto-backport this PR.~~

The PR was tested like this:

Clone the repo, apply the patch and build a new `crank` binary:

```shell
cd /tmp
gh repo clone crossplane/crossplane
cd crossplane
curl -s https://patch-diff.githubusercontent.com/raw/crossplane/crossplane/pull/5669.patch | git apply
make submodules
make go.build
```

Create directory structure for the test:

```shell
mkdir -p /tmp/test/{apis/foo.custom-api.example.org,test/{functions,validation}}
cd /tmp/test
```

Create files:

```shell
# Configuration for the "foo" package
cat <<END > apis/configuration.yaml
apiVersion: meta.pkg.crossplane.io/v1alpha1
kind: Configuration
metadata:
  name: foo
spec:
  dependsOn:
    - function: xpkg.upbound.io/crossplane-contrib/function-patch-and-transform
      version: ">=v0.4.0"
    - provider: xpkg.upbound.io/upbound/provider-aws-s3
      version: ">=v1.3.1"
  crossplane:
    version: ">=v1.15.2"
END

# Foo composition
cat <<END > apis/foo.custom-api.example.org/composition.yaml
apiVersion: apiextensions.crossplane.io/v1
kind: Composition
metadata:
  name: xfoos.custom-api.example.org
spec:
  compositeTypeRef:
    apiVersion: custom-api.example.org/v1alpha1
    kind: XFoo
  mode: Pipeline
  pipeline:
    - step: patch-and-transform
      functionRef:
        name: crossplane-contrib-function-patch-and-transform
      input:
        apiVersion: pt.fn.crossplane.io/v1beta1
        kind: Resources
        resources:
          - name: storage-bucket
            base:
              apiVersion: s3.aws.upbound.io/v1beta1
              kind: Bucket
              spec:
                forProvider:
                  region:  # Patched
            patches:
              - type: FromCompositeFieldPath
                fromFieldPath: spec.location
                toFieldPath: spec.forProvider.region
END

# XRD for the Foo composition
cat <<END > apis/foo.custom-api.example.org/definition.yaml
apiVersion: apiextensions.crossplane.io/v1
kind: CompositeResourceDefinition
metadata:
  name: xfoos.custom-api.example.org
spec:
  group: custom-api.example.org
  names:
    kind: XFoo
    plural: xfoos
  claimNames:
    kind: Foo
    plural: Foos
  versions:
    - name: v1alpha1
      schema:
        openAPIV3Schema:
          type: object
          required:
            - spec
          properties:
            spec:
              type: object
              required:
                - location
              properties:
                location:
                  type: string
END

cat <<END > test/claim.yaml
apiVersion: custom-api.example.org/v1alpha1
kind: Bar
metadata:
  name: mybar
spec:
  location: us-east-2
END

# Bar composition
cat <<END > test/composition.yaml
apiVersion: apiextensions.crossplane.io/v1
kind: Composition
metadata:
  name: bar
spec:
  compositeTypeRef:
    apiVersion: custom-api.example.org/v1alpha1
    kind: Bar
  mode: Pipeline
  pipeline:
    - step: patch-and-transform
      functionRef:
        name: crossplane-contrib-function-patch-and-transform
      input:
        apiVersion: pt.fn.crossplane.io/v1beta1
        kind: Resources
        resources:
          - name: foo
            base:
              apiVersion: custom-api.example.org/v1alpha1
              kind: Foo
              spec:
                location:  # Patched
            patches:
              - type: FromCompositeFieldPath
                fromFieldPath: spec.location
                toFieldPath: spec.location
END

# Function used bu the Bar composition
cat <<END > test/functions/patch-and-transform.yaml
apiVersion: pkg.crossplane.io/v1beta1
kind: Function
metadata:
  name: crossplane-contrib-function-patch-and-transform
spec:
  package: xpkg.upbound.io/crossplane-contrib/function-patch-and-transform:v0.4.0
END

# Configuration for the "foo" package
cat <<END > test/validation/configuration.yaml
apiVersion: pkg.crossplane.io/v1beta1
kind: Configuration
metadata:
  name: foo
spec:
  package: localhost:5000/foo:v0.1.0
END

# XRD for the Bar composition
cat <<END > test/validation/definition.yaml
apiVersion: apiextensions.crossplane.io/v1
kind: CompositeResourceDefinition
metadata:
  name: xbar.custom-api.example.org
spec:
  group: custom-api.example.org
  names:
    kind: XBar
    plural: xbars
  claimNames:
    kind: Bar
    plural: Bars
  versions:
    - name: v1alpha1
      schema:
        openAPIV3Schema:
          type: object
          required:
            - spec
          properties:
            spec:
              type: object
              required:
                - location
              properties:
                location:
                  type: string
END
```

Run local Docker registry:

```shell
docker run -d -p 5000:5000 --restart always --name registry registry:2
```

Package and push the `foo` package:

```shell
crossplane xpkg build --package-root apis --package-file foo.xpkg
crossplane xpkg push --package-files foo.xpkg localhost:5000/foo:v0.1.0
```

Validate the `Bar` composition using the upstream `crank` binary:

```shell
$ crossplane beta render test/claim.yaml test/composition.yaml test/functions --include-full-xr | crossplane beta validate test/validation -
package schemas does not exist, downloading:  xpkg.upbound.io/crossplane-contrib/function-patch-and-transform:>=v0.4.0
crossplane: error: cannot download and load cache: cannot cache package dependencies: cannot download package xpkg.upbound.io/crossplane-contrib/function-patch-and-transform:>=v0.4.0: cannot get config: parsing reference "xpkg.upbound.io/crossplane-contrib/function-patch-and-transform:>=v0.4.0": could not parse reference: xpkg.upbound.io/crossplane-contrib/function-patch-and-transform:>=v0.4.0
```

Validate the `Bar` composition using the patched `crank` binary:

```shell
$ crossplane beta render test/claim.yaml test/composition.yaml test/functions --include-full-xr | /tmp/crossplane/_output/bin/linux_amd64/crank beta validate test/validation -
package schemas does not exist, downloading:  xpkg.upbound.io/upbound/provider-aws-s3:>=v1.3.1
package schemas does not exist, downloading:  xpkg.upbound.io/crossplane-contrib/function-patch-and-transform:>=v0.4.0
[✓] custom-api.example.org/v1alpha1, Kind=Bar, mybar validated successfully
[✓] custom-api.example.org/v1alpha1, Kind=Foo, foo validated successfully
Total 2 resources: 0 missing schemas, 2 success cases, 0 failure cases
```

jtyr: @phisco @negz @jbw976 Please could you review this PR? It would be great if it would make it into the v1.16.0 release :crossed_fingers:
negz: @jtyr Unfortunately this won't make it into v1.16 - feature freeze for that release was two weeks ago and it's being released tomorrow. Sorry!
jtyr: @phisco @negz @jbw976 Please could you review this PR? It would be great if it would make it into the next release 🤞
negz: Perhaps @ezgidemirel would be a good first reviewer for this, given she implemented the `validate` subcommand.
jtyr: @ezgidemirel please could your review this PR?

Patch:
diff --git a/cmd/crank/beta/validate/image.go b/cmd/crank/beta/validate/image.go
index a308bb426..8cc0e50e1 100644
--- a/cmd/crank/beta/validate/image.go
+++ b/cmd/crank/beta/validate/image.go
@@ -18,8 +18,10 @@ package validate
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
+	"github.com/Masterminds/semver"
 	"github.com/google/go-containerregistry/pkg/crane"
 	conregv1 "github.com/google/go-containerregistry/pkg/v1"
 	"k8s.io/apimachinery/pkg/util/yaml"
@@ -37,8 +39,15 @@ type Fetcher struct{}
 
 // FetchBaseLayer fetches the base layer of the image which contains the 'package.yaml' file.
 func (f *Fetcher) FetchBaseLayer(image string) (*conregv1.Layer, error) {
-	if strings.Contains(image, "sha") { // Strip the digest before fetching the image
-		image = strings.Split(image, "@")[0]
+	if strings.Contains(image, "@") {
+		// Strip the digest before fetching the image
+		image = strings.SplitN(image, "@", 2)[0]
+	} else if strings.Contains(image, ":") {
+		var err error
+		image, err = findImageTagForVersionConstraint(image)
+		if err != nil {
+			return nil, errors.Wrapf(err, "cannot find image tag for version constraint")
+		}
 	}
 
 	cBytes, err := crane.Config(image)
@@ -74,6 +83,63 @@ func (f *Fetcher) FetchBaseLayer(image string) (*conregv1.Layer, error) {
 	return &ll, nil
 }
 
+func findImageTagForVersionConstraint(image string) (string, error) {
+	// Separate the image base and the image tag
+	parts := strings.Split(image, ":")
+	lastPart := len(parts) - 1
+	imageBase := strings.Join(parts[0:lastPart], ":")
+	imageTag := parts[lastPart]
+
+	// Check if the tag is a constraint
+	isConstraint := true
+	c, err := semver.NewConstraint(imageTag)
+	if err != nil {
+		isConstraint = false
+	}
+
+	// Return original image if no constraint was detected
+	if !isConstraint {
+		return image, nil
+	}
+
+	// Fetch all image tags
+	tags, err := crane.ListTags(imageBase)
+	if err != nil {
+		return "", errors.Wrapf(err, "cannot fetch tags for the image %s", imageBase)
+	}
+
+	// Convert tags to semver versions
+	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 all versions and find the last version complient with the constraint
+	sort.Sort(sort.Reverse(semver.Collection(vs)))
+	var addVer string
+	for _, v := range vs {
+		if c.Check(v) {
+			addVer = v.Original()
+
+			break
+		}
+	}
+
+	if addVer == "" {
+		return "", errors.Errorf("cannot find any tag complient with the constraint %s", imageTag)
+	}
+
+	// Compose new complete image string if any complient version was found
+	image = fmt.Sprintf("%s:%s", imageBase, addVer)
+
+	return image, nil
+}
+
 func extractPackageContent(layer conregv1.Layer) ([][]byte, []byte, error) {
 	rc, err := layer.Uncompressed()
 	if err != nil {


Test patch:
diff --git a/cmd/crank/beta/validate/image_test.go b/cmd/crank/beta/validate/image_test.go
new file mode 100644
index 000000000..406478b0f
--- /dev/null
+++ b/cmd/crank/beta/validate/image_test.go
@@ -0,0 +1,113 @@
+/*
+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 validate
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+)
+
+func TestFindImageTagForVersionConstraint(t *testing.T) {
+	repoName := "ubuntu"
+	responseTags := []byte(`{"tags":["1.2.3","4.5.6"]}`)
+	cases := map[string]struct {
+		responseBody  []byte
+		host          string
+		constraint    string
+		expectedImage string
+		expectError   bool
+	}{
+		"NoConstraint": {
+			responseBody:  responseTags,
+			constraint:    "1.2.3",
+			expectedImage: "ubuntu:1.2.3",
+		},
+		"Constraint": {
+			responseBody:  responseTags,
+			constraint:    ">=1.2.3",
+			expectedImage: "ubuntu:4.5.6",
+		},
+		"ConstraintV": {
+			responseBody:  responseTags,
+			constraint:    ">=v1.2.3",
+			expectedImage: "ubuntu:4.5.6",
+		},
+		"ConstraintPreRelease": {
+			responseBody:  responseTags,
+			constraint:    ">v4.5.6-rc.0.100.g658deda0.dirty",
+			expectedImage: "ubuntu:4.5.6",
+		},
+		"CannotFetchTags": {
+			responseBody: responseTags,
+			host:         "wrong.host",
+			constraint:   ">=4.5.6",
+			expectError:  true,
+		},
+		"NoMatchingTag": {
+			responseBody: responseTags,
+			constraint:   ">4.5.6",
+			expectError:  true,
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			tagsPath := fmt.Sprintf("/v2/%s/tags/list", repoName)
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				switch r.URL.Path {
+				case "/v2/":
+					w.WriteHeader(http.StatusOK)
+				case tagsPath:
+					if r.Method != http.MethodGet {
+						t.Errorf("Method; got %v, want %v", r.Method, http.MethodGet)
+					}
+
+					w.Write(tc.responseBody)
+				default:
+					t.Fatalf("Unexpected path: %v", r.URL.Path)
+				}
+			}))
+			defer server.Close()
+
+			u, err := url.Parse(server.URL)
+			if err != nil {
+				t.Fatalf("url.Parse(%v) = %v", server.URL, err)
+			}
+
+			host := u.Host
+			if tc.host != "" {
+				host = tc.host
+			}
+
+			image, err := findImageTagForVersionConstraint(fmt.Sprintf("%s/%s:%s", host, repoName, tc.constraint))
+
+			expectedImage := ""
+			if !tc.expectError {
+				expectedImage = fmt.Sprintf("%s/%s", host, tc.expectedImage)
+			}
+
+			if tc.expectError && err == nil {
+				t.Errorf("[%s] expected: error\n", name)
+			} else if expectedImage != image {
+				t.Errorf("[%s] expected: %s, got: %s\n", name, expectedImage, image)
+			}
+		})
+	}
+}