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

Upgrade to Go 1.23 and golang.org/x to 0.21.0 #278

Merged
merged 2 commits into from
Oct 1, 2024

Conversation

flostadler
Copy link
Contributor

@flostadler flostadler commented Oct 1, 2024

This is necessary for upgrade-provider to detect new godebug directives like tlskyber.
That godebug directive is used in pulumi-aws to work around an issue with AWS firewalls. Right now upgrade-provider fails when parsing the go.mod file because it doesn't know about the tlskyber directive.

I verified this correctly works by executing an upstream check on the aws repo:

GH_TOKEN="$(gh auth token)" ./../upgrade-provider/bin/upgrade-provider "pulumi/pulumi-aws" --repo-path . --upstream-provider-name terraform-provider-aws --kind=check-upstream-version

Relates to pulumi/pulumi-aws#4586

This is necessary for upgrade-provider to detect new godebug
directives like `tlskyber`.
@flostadler flostadler requested a review from a team October 1, 2024 09:20
@flostadler flostadler self-assigned this Oct 1, 2024
@flostadler flostadler marked this pull request as draft October 1, 2024 09:23
@iwahbe
Copy link
Member

iwahbe commented Oct 1, 2024

I think you need to bump the linter version as well.

@@ -29,7 +29,9 @@ type httpHandler interface {
getHTTP(url string) ([]byte, error)
}

var httpHandlerKey = struct{}{}
type httpContextKey struct{}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addresses linter warning SA1029: should not use empty anonymous struct as key for value; define your own type to avoid collisions

@@ -24,7 +24,7 @@ const (
func AutoAliasingMigration(resourcesFilePath, providerName string) (bool, error) {
// Create the AST by parsing src
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, resourcesFilePath, nil, parser.ParseComments)
file, err := parser.ParseFile(fset, resourcesFilePath, nil, parser.ParseComments|parser.SkipObjectResolution)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntactic object resolution is deprecated now including ast.Object, see: https://pkg.go.dev/go/ast#Object.

We don't need object resolution because we're doing ast manipulation and not analysis or resolving symbols across scopes.

@@ -96,7 +96,7 @@ func AutoAliasingMigration(resourcesFilePath, providerName string) (bool, error)
}
c.InsertBefore(&ast.AssignStmt{
Tok: tok,
Lhs: []ast.Expr{&ast.Ident{Name: "err", Obj: &ast.Object{Kind: ast.Var, Name: "err"}}},
Lhs: []ast.Expr{&ast.Ident{Name: "err"}},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ident.Obj is deprecated now. It wasn't needed for our use cases here.

@@ -202,7 +202,7 @@ func UpgradeProvider(ctx context.Context, repoOrg, repoName string) (err error)
return err
}
defer func() {
fmt.Printf("\n\n" + colorize.Warn("Major Version Updates are not fully automated!") + "\n")
fmt.Printf("\n\n%s\n", colorize.Warn("Major Version Updates are not fully automated!"))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed: non-constant format string in call to fmt.Printf

@@ -98,7 +98,7 @@ var getRepoKind = stepv2.Func11E("Get Repo Kind", func(ctx context.Context, repo
if err != nil {
return nil, fmt.Errorf("%s: %w", bridgeMissingMsg, err)
} else if !ok {
return nil, fmt.Errorf(bridgeMissingMsg)
return nil, fmt.Errorf("%s", bridgeMissingMsg)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed: non-constant format string in call to fmt.Errorf

@flostadler flostadler marked this pull request as ready for review October 1, 2024 10:43
@flostadler flostadler requested review from iwahbe and a team October 1, 2024 10:43
@flostadler
Copy link
Contributor Author

@iwahbe can you have another look please? I had to address some deprecations and new linter warnings

Copy link
Member

@iwahbe iwahbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the lints.

@flostadler flostadler merged commit 0a17467 into main Oct 1, 2024
2 checks passed
@flostadler flostadler deleted the flostadler/upgrade-gomod-dep branch October 1, 2024 11:12
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.

2 participants