Pulumi-language-go and pulumi new now checks go version is at least 1.14.0 #5741

viveklak · 2020-11-11T23:33:36Z

Fixes #5724
This change enforces that the go version is at least 1.14.0 for go projects.

Now the following error is seen when running pulumi up on an incompatible version:

# pulumi up
Previewing update (dev)

View Live: https://app.pulumi.com/.../dev/previews/...


error: failed to discover plugin requirements: go version must be 1.14.0 or higher (1.13.8 detected)

and the following when pulumi new is invoked:

pulumi new gcp-go

Enter a value or leave blank to accept the (default), and press <ENTER>.
Press ^C at any time to quit.

project name: (new-project)
project description: (A minimal Google Cloud Go Pulumi program)
Created project 'new-project'

Please enter your desired stack name.
To create a stack in an organization, use the format <org-name>/<stack-name> (e.g. `acmecorp/dev`).
stack name: (dev)
Created stack 'dev'

gcp:project: The Google Cloud project to deploy into:
Saved config

Installing dependencies...

error: go version must be 1.14.0 or higher (1.13.8 detected)

viveklak · 2020-11-11T23:43:13Z

sdk/go/pulumi-language-go/main.go

-		// will err if the project isn't using modules
-		logging.V(5).Infof("GetRequiredPlugins: Error discovering plugin requirements: %s", err.Error())
-		return &pulumirpc.GetRequiredPluginsResponse{}, nil
+		logging.V(5).Infof("GetRequiredPlugins: Error discovering plugin requirements using go modules: %s", err.Error())


Previously returning an empty response essentially silently disabled plugin acquisition for users with glide or dep projects. We can reinstate the behavior if we feel we aren't ready for this breaking change.

I'm leaning towards not introducing this break just yet.

justinvp

I'm leaning towards continuing to not error on the go list command, at least for now.

Aside from that and some nits, LGTM.

justinvp · 2020-11-12T00:49:04Z

sdk/go/pulumi-language-go/main.go

+	cmd := exec.Command(gobin, "version")
+	stdout, err := cmd.Output()
+	if err != nil {
+		return nil, fmt.Errorf("failed to determine go version: %w", err)


Nit:

Suggested change

return nil, fmt.Errorf("failed to determine go version: %w", err)

return nil, errors.Wrap(err, "determining go version")

justinvp · 2020-11-12T00:50:29Z

sdk/go/pulumi-language-go/main.go

+func checkMinimumGoVersion(goVersionOutput string) error {
+	split := strings.Split(goVersionOutput, " ")
+	if len(split) <= 2 {
+		return fmt.Errorf("unexpected format for go version output: \"%s\"", goVersionOutput)


Nit:

Suggested change

return fmt.Errorf("unexpected format for go version output: \"%s\"", goVersionOutput)

return errors.Errorf("unexpected format for go version output: \"%s\"", goVersionOutput)

justinvp · 2020-11-12T00:51:13Z

sdk/go/pulumi-language-go/main.go

+
+	currVersion, err := semver.ParseTolerant(version)
+	if err != nil {
+		return fmt.Errorf("parsing go version failed: %w", err)


Nit:

Suggested change

return fmt.Errorf("parsing go version failed: %w", err)

return errors.Wrap(err, "parsing go version")

justinvp · 2020-11-12T00:52:12Z

sdk/go/pulumi-language-go/main.go

+	}
+
+	if currVersion.LT(minGoVersion) {
+		return fmt.Errorf("go version must be %s or higher (%s detected)", minGoVersion.String(), version)


Nit:

Suggested change

return fmt.Errorf("go version must be %s or higher (%s detected)", minGoVersion.String(), version)

return errors.Errorf("go version must be %s or higher (%s detected)", minGoVersion.String(), version)

justinvp · 2020-11-12T00:55:08Z

sdk/go/pulumi-language-go/main.go

-		// will err if the project isn't using modules
-		logging.V(5).Infof("GetRequiredPlugins: Error discovering plugin requirements: %s", err.Error())
-		return &pulumirpc.GetRequiredPluginsResponse{}, nil
+		logging.V(5).Infof("GetRequiredPlugins: Error discovering plugin requirements using go modules: %s", err.Error())


I'm leaning towards not introducing this break just yet.

viveklak · 2020-11-12T06:27:25Z

I'm leaning towards continuing to not error on the go list command, at least for now.

Aside from that and some nits, LGTM.

I decided to factor out the version check since I wanted to enforce this on pulumi new as well. I have applied the nit improvements you mentioned and have also excluded the "breaking" change. Apologies for the churn but hopefully the change is easy to grok.

justinvp

LGTM

Pulumi-language-go now checks go version is atleast 1.14.0

f3b41e0

viveklak force-pushed the vl/GoVersion branch from a56bb7b to cca1d69 Compare November 11, 2020 23:40

viveklak commented Nov 11, 2020

View reviewed changes

viveklak requested a review from justinvp November 11, 2020 23:50

justinvp approved these changes Nov 12, 2020

View reviewed changes

viveklak force-pushed the vl/GoVersion branch from cca1d69 to aa91d89 Compare November 12, 2020 06:27

viveklak changed the title ~~Pulumi-language-go now checks go version is atleast 1.14.0~~ Pulumi-language-go and pulumi new now checks go version is at least 1.14.0 Nov 12, 2020

Vivek Lakshmanan added 2 commits November 12, 2020 09:20

Refactor to add version check to pulumi new as well

bcb9d0f

Fix test

b2e2562

viveklak force-pushed the vl/GoVersion branch from 2d64e3f to b2e2562 Compare November 12, 2020 17:20

justinvp approved these changes Nov 12, 2020

View reviewed changes

viveklak merged commit 9d6e380 into master Nov 12, 2020

pulumi-bot deleted the vl/GoVersion branch November 12, 2020 23:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pulumi-language-go and pulumi new now checks go version is at least 1.14.0 #5741

Pulumi-language-go and pulumi new now checks go version is at least 1.14.0 #5741

viveklak commented Nov 11, 2020 •

edited

Loading

viveklak Nov 11, 2020

justinvp Nov 12, 2020

justinvp left a comment

justinvp Nov 12, 2020

justinvp Nov 12, 2020

justinvp Nov 12, 2020

justinvp Nov 12, 2020

justinvp Nov 12, 2020

viveklak commented Nov 12, 2020

justinvp left a comment

	return nil, fmt.Errorf("failed to determine go version: %w", err)
	return nil, errors.Wrap(err, "determining go version")

	return fmt.Errorf("unexpected format for go version output: \"%s\"", goVersionOutput)
	return errors.Errorf("unexpected format for go version output: \"%s\"", goVersionOutput)

	return fmt.Errorf("parsing go version failed: %w", err)
	return errors.Wrap(err, "parsing go version")

	return fmt.Errorf("go version must be %s or higher (%s detected)", minGoVersion.String(), version)
	return errors.Errorf("go version must be %s or higher (%s detected)", minGoVersion.String(), version)

Pulumi-language-go and pulumi new now checks go version is at least 1.14.0 #5741

Pulumi-language-go and pulumi new now checks go version is at least 1.14.0 #5741

Conversation

viveklak commented Nov 11, 2020 • edited Loading

viveklak Nov 11, 2020

Choose a reason for hiding this comment

justinvp Nov 12, 2020

Choose a reason for hiding this comment

justinvp left a comment

Choose a reason for hiding this comment

justinvp Nov 12, 2020

Choose a reason for hiding this comment

justinvp Nov 12, 2020

Choose a reason for hiding this comment

justinvp Nov 12, 2020

Choose a reason for hiding this comment

justinvp Nov 12, 2020

Choose a reason for hiding this comment

justinvp Nov 12, 2020

Choose a reason for hiding this comment

viveklak commented Nov 12, 2020

justinvp left a comment

Choose a reason for hiding this comment

viveklak commented Nov 11, 2020 •

edited

Loading