crossplane-contrib / provider-upjet-aws

Official AWS Provider for Crossplane by Upbound.
https://marketplace.upbound.io/providers/upbound/provider-aws
Apache License 2.0
137 stars 113 forks source link

pin aws-sdk-go-v2 dependency versions for ClusterAuth presign breaking change #1251

Closed erhancagirici closed 3 months ago

erhancagirici commented 3 months ago

Description of your changes

Fixes #1248 Recent patch version bumps at the following aws-sdk-go-v2 modules seems to break the request signature of the token for the generated kubeconfig for ClusterAuth resource.

github.com/aws/aws-sdk-go-v2
github.com/aws/aws-sdk-go-v2/internal/configsources
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2

This change reverts the version bumps and pins the versions. Adds a separate standalone example manifest for ClusterAuth resource and added an Uptest post-assert hook for testing the resulting kubeconfig, by executing simple kubectl commands to ensure the cluster authentication.

I have:

How has this code been tested

jeanduplessis commented 3 months ago

Is it possible to add a test to catch a regression here in the future? Is ClusterAuth already uptestable and we should consider it a critical component that should always be tested before a release?

erhancagirici commented 3 months ago

Is it possible to add a test to catch a regression here in the future? Is ClusterAuth already uptestable and we should consider it a critical component that should always be tested before a release?

ClusterAuth is already uptestable, and included in the examples/v1beta1/cluster.yaml. The MR actually gets created, switches to ready & synced state, and the connection details are published. In order to catch the regression here, the resulting kubeconfig in the connection details should be consumed.

We might consider utilizing https://github.com/upbound/configuration-aws-eks, some test MRs utilizing provider-kubernetes or some post-test script utilizing kubectl. cc @ulucinar @sergenyalcin

ulucinar commented 3 months ago

Hi @erhancagirici, We may consider utilizing an uptest post-assert-hook for ClusterAuth.eks in which we run a script that:

Could you please also trigger an uptest on ClusterAuth.eks with these modifications? We may also consider adding the example manifest examples/eks/v1beta1/clusterauth.yaml.

erhancagirici commented 3 months ago

/test-examples="examples/eks/v1beta1/clusterauth.yaml"

erhancagirici commented 3 months ago

/test-examples="examples/eks/v1beta1/clusterauth.yaml"

erhancagirici commented 3 months ago

/test-examples="examples/eks/v1beta1/clusterauth.yaml"

github-actions[bot] commented 3 months ago

Successfully created backport PR #1257 for release-1.3.

mbbush commented 2 months ago

It looks like this was probably caused by https://github.com/aws/aws-sdk-go-v2/pull/2438. I see a bunch of other projects citing that PR as an issue too. I don't understand why yet.