Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 10 commits into from

Conversation

mbbush
Copy link
Collaborator

@mbbush mbbush commented Apr 21, 2024

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:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

@mbbush
Copy link
Collaborator Author

mbbush commented Apr 21, 2024

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

@mbbush
Copy link
Collaborator Author

mbbush commented Apr 21, 2024

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

@mbbush
Copy link
Collaborator Author

mbbush commented Apr 21, 2024

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

@mbbush
Copy link
Collaborator Author

mbbush commented Apr 21, 2024

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
Copy link
Collaborator Author

mbbush commented Apr 21, 2024

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

Error: ../../../go/pkg/mod/github.com/upbound/[email protected]/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 aws/aws-sdk-go-v2#2458

@mbbush
Copy link
Collaborator Author

mbbush commented Apr 21, 2024

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 upbound/terraform-provider-aws#196 into my fork correctly? Or maybe the change is incompatible with something done in main on the terraform provider?

@mbbush
Copy link
Collaborator Author

mbbush commented Apr 21, 2024

hashicorp/terraform-provider-aws@b87c156 This is why there's the error on Session

@mbbush
Copy link
Collaborator Author

mbbush commented Apr 21, 2024

@mergenci Any ideas why af4029c doesn't compile? I'm puzzled because I see essentially the same signature for other getters like hashicorp/terraform-provider-aws@b87c156#diff-2bef3ec311c0fa59f07b6d4b0f38154f51e02a7c4387384e70d8874bd1628d65R57 which I can use without problems.

@mergenci
Copy link
Collaborator

@mbbush, After conducting some source-code archeology, I unearthed that af4029c doesn't compile, because conns.AWSClient.Session(), introduced in hashicorp/terraform-provider-aws@b87c156, was removed in the next commit, hashicorp/terraform-provider-aws@363e6a4 (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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants