search

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

[do not merge] revert aws go sdk pinning to see if I can simulate the breakage #1274

Closed mbbush closed 2 months ago

mbbush commented 2 months ago

Description of your changes

It looks like the solution to the clusterauth issue of pinning the go sdk breaks upgrading the terraform provider. I'm using this PR to experiment with alternative solutions.

Fixes #

I have:

How has this code been tested

mbbush commented 2 months ago

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

mbbush commented 2 months ago

/test-examples="examples/eks/v1beta1/clusterauth.yaml,examples/ec2/v1beta1/instance.yaml"

mbbush commented 2 months ago

/test-examples="examples/eks/v1beta1/clusterauth.yaml,examples/ec2/v1beta1/instance.yaml"

mbbush commented 2 months ago

https://github.com/crossplane-contrib/provider-upjet-aws/actions/runs/8770461647/job/24067041363 using aws sdk v1.24.1 is failing on the clusterauth step as expected.


2024-04-21T06:32:48.1063688Z     logger.go:42: 06:32:48 | case/0-apply | obtain kubeconfig from ClusterAuth connection secret
2024-04-21T06:32:48.1561719Z     logger.go:42: 06:32:48 | case/0-apply | checking kubectl version
2024-04-21T06:32:48.3952892Z     logger.go:42: 06:32:48 | case/0-apply | WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
2024-04-21T06:32:48.3958301Z     logger.go:42: 06:32:48 | case/0-apply | Client Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.3", GitCommit:"aef86a93758dc3cb2c658dd9657ab4ad4afc21cb", GitTreeState:"clean", BuildDate:"2022-07-13T14:30:46Z", GoVersion:"go1.18.3", Compiler:"gc", Platform:"linux/amd64"}
2024-04-21T06:32:48.3960094Z     logger.go:42: 06:32:48 | case/0-apply | Kustomize Version: v4.5.4
2024-04-21T06:32:48.3961550Z     logger.go:42: 06:32:48 | case/0-apply | error: You must be logged in to the server (the server has asked for the client to provide credentials)
mbbush commented 2 months ago

Simply upgrading to the latest sdk with no other changes fails to build with

Error: ../../../go/pkg/mod/github.com/upbound/terraform-provider-aws@v0.0.0-20240328111213-f2f0fdd63866/internal/service/emrserverless/application.go:731:27: cannot use int64(v) (value of type int64) as *int64 value in assignment

This is probably a result of https://github.com/aws/aws-sdk-go-v2/pull/2458

mbbush commented 2 months ago

Upgrading to the latest tf provider version fails to compile with 

# github.com/upbound/provider-aws/internal/clients
internal/clients/aws.go:243:19: tfAwsConnsClient.Session undefined (type *conns.AWSClient has no field or method Session)

Maybe I didn't include https://github.com/upbound/terraform-provider-aws/pull/196 into my fork correctly? Or maybe the change is incompatible with something done in main on the terraform provider?

mbbush commented 2 months ago

https://github.com/hashicorp/terraform-provider-aws/commit/b87c156fa3ac1199937815266d6f839b509572f3 This is why there's the error on Session

mbbush commented 2 months ago

@mergenci Any ideas why https://github.com/crossplane-contrib/provider-upjet-aws/pull/1274/commits/af4029c8246fd4d15c1f5f8e97b233bcaa39883c doesn't compile? I'm puzzled because I see essentially the same signature for other getters like https://github.com/hashicorp/terraform-provider-aws/commit/b87c156fa3ac1199937815266d6f839b509572f3#diff-2bef3ec311c0fa59f07b6d4b0f38154f51e02a7c4387384e70d8874bd1628d65R57 which I can use without problems.

mergenci commented 2 months ago

@mbbush, After conducting some source-code archeology, I unearthed that https://github.com/crossplane-contrib/provider-upjet-aws/commit/af4029c8246fd4d15c1f5f8e97b233bcaa39883c doesn't compile, because conns.AWSClient.Session(), introduced in https://github.com/hashicorp/terraform-provider-aws/commit/b87c156fa3ac1199937815266d6f839b509572f3, was removed in the next commit, https://github.com/hashicorp/terraform-provider-aws/commit/363e6a4d459532a3c467cda0c73999bb2fd74091 (see file history). Current version of awsclient.go, commit ID of which (0ec53cbccd61) is referenced from go.mod, doesn't have the getter.

It looks like it should be safe to introduce the getter to awsclient_xp.go and have everything working as expected, but I haven't tested.