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

pulumi convert to Go generates incorrect code and go.mod for component usage #11427

Closed
lukehoban opened this issue Oct 29, 2022 · 4 comments
Closed
Assignees
Labels
area/codegen SDK-gen, program-gen, convert kind/bug Some behavior is incorrect or out of spec language/go resolution/fixed This issue was fixed
Milestone

Comments

@lukehoban
Copy link
Member

Creating the default pulumi new serverless-aws-yaml and then pulumi convert --language go --out ./go fails with this:

Installing dependencies...

go: finding module for package github.com/pulumi/pulumi-aws-apigateway/sdk/apigateway/index
go: finding module for package github.com/pulumi/pulumi-aws-apigateway/sdk/apigateway
demo2 imports
        github.com/pulumi/pulumi-aws-apigateway/sdk/apigateway: module github.com/pulumi/pulumi-aws-apigateway/sdk@latest found (v0.0.10), but does not contain package github.com/pulumi/pulumi-aws-apigateway/sdk/apigateway
demo2 imports
        github.com/pulumi/pulumi-aws-apigateway/sdk/apigateway/index: module github.com/pulumi/pulumi-aws-apigateway/sdk@latest found (v0.0.10), but does not contain package github.com/pulumi/pulumi-aws-apigateway/sdk/apigateway/index
error: installing dependencies failed; rerun manually to try again, then run `pulumi up` to perform an initial deployment: `go mod tidy` failed to install dependencies: exit status 1

The code it generates is also wrong in referencing github.com/pulumi/pulumi-aws-apigateway/sdk/apigateway/index and in using unqualified references to types from apigateway like RouteArgs.

package main

import (
	"encoding/json"

	"github.com/pulumi/pulumi-aws-apigateway/sdk/apigateway"
	"github.com/pulumi/pulumi-aws-apigateway/sdk/apigateway/index"
	"github.com/pulumi/pulumi-aws/sdk/v5/go/aws"
	"github.com/pulumi/pulumi-aws/sdk/v5/go/aws/iam"
	"github.com/pulumi/pulumi-aws/sdk/v5/go/aws/lambda"
	"github.com/pulumi/pulumi/sdk/v3/go/pulumi"
)

func main() {
	pulumi.Run(func(ctx *pulumi.Context) error {
		tmpJSON0, err := json.Marshal(map[string]interface{}{
			"Version": "2012-10-17",
			"Statement": []map[string]interface{}{
				map[string]interface{}{
					"Action": "sts:AssumeRole",
					"Effect": "Allow",
					"Principal": map[string]interface{}{
						"Service": "lambda.amazonaws.com",
					},
				},
			},
		})
		if err != nil {
			return err
		}
		json0 := string(tmpJSON0)
		role, err := iam.NewRole(ctx, "role", &iam.RoleArgs{
			AssumeRolePolicy: pulumi.String(json0),
			ManagedPolicyArns: pulumi.StringArray{
				pulumi.String("arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"),
			},
		})
		if err != nil {
			return err
		}
		fn, err := lambda.NewFunction(ctx, "fn", &lambda.FunctionArgs{
			Runtime: pulumi.String("python3.8"),
			Handler: pulumi.String("handler.handler"),
			Role:    role.Arn,
			Code:    pulumi.NewFileArchive("./function"),
		})
		if err != nil {
			return err
		}
		api, err := apigateway.NewRestAPI(ctx, "api", &apigateway.RestAPIArgs{
			Routes: []RouteArgs{
				&RouteArgs{
					Path:      "/",
					LocalPath: "www",
				},
				&RouteArgs{
					Path:         "/date",
					Method:       index.MethodGET,
					EventHandler: fn.ID(),
				},
			},
		})
		if err != nil {
			return err
		}
		ctx.Export("url", api.Url)
		return nil
	})
}

The YAML source code from the default template is this:

name: demo2
description: A serverless app on AWS API Gateway and Lambda
runtime: yaml
resources:
    # An execution role to use for the Lambda function
    role:
        type: aws:iam:Role
        properties:
            assumeRolePolicy:
                fn::toJSON:
                    Version: 2012-10-17
                    Statement:
                        - Action: sts:AssumeRole
                          Effect: Allow
                          Principal:
                            Service: lambda.amazonaws.com
            managedPolicyArns:
                - arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole
    # A Lambda function to invoke
    fn:
        type: aws:lambda:Function
        properties:
            runtime: python3.8
            handler: handler.handler
            role: ${role.arn}
            code:
                fn::fileArchive: ./function
    # A REST API to route requests to HTML content and the Lambda function
    api:
        type: aws-apigateway:RestAPI
        properties:
            routes:
                - path: /
                  localPath: www
                - path: /date
                  method: GET
                  eventHandler: ${fn}
outputs:
    # The URL at which the REST API will be served.
    url: ${api.url}
@lukehoban lukehoban added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Oct 29, 2022
@iwahbe iwahbe added area/codegen SDK-gen, program-gen, convert and removed needs-triage Needs attention from the triage team labels Oct 31, 2022
@iwahbe iwahbe self-assigned this Nov 14, 2022
@iwahbe iwahbe added this to the 0.81 milestone Nov 14, 2022
@iwahbe iwahbe transferred this issue from pulumi/pulumi-yaml Nov 22, 2022
@iwahbe
Copy link
Member

iwahbe commented Nov 22, 2022

I've transferred the issue from pulumi-yaml to pu/pu. The generated PCL looks correct, and other languages generate valid code. I think this is a problem with the go language generator.

Generated PCL:

resource role "aws:iam/role:Role" {
	__logicalName = "role"
	assumeRolePolicy = toJSON({
		"Version" = "2012-10-17",
		"Statement" = [{
			"Action" = "sts:AssumeRole",
			"Effect" = "Allow",
			"Principal" = {
				"Service" = "lambda.amazonaws.com"
			}
		}]
	})
	managedPolicyArns = ["arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"]
}

resource fn "aws:lambda/function:Function" {
	__logicalName = "fn"
	runtime = "python3.9"
	handler = "handler.handler"
	role = role.arn
	code = fileArchive("./function")
}

resource api "aws-apigateway:index:RestAPI" {
	__logicalName = "api"
	routes = [
		{
			path = "/",
			localPath = "www"
		},
		{
			path = "/date",
			method = "GET",
			eventHandler = fn
		}
	]
}

output url {
	__logicalName = "url"
	value = api.url
}

bors bot added a commit that referenced this issue Nov 22, 2022
11431: Bumps grpcio to version 1.50 r=kpitzen a=kpitzen

Currently, pinning to a version of grpcio <1.50 can cause build errors on newer versions of python+pip - the existing setup.py install method of building libararies is being deprecated and wheels are not being backported for older versions of grpcio.  Since this change is a no-op as far as our python proto clients are concerned, it feels safe to bump to a version that will be supported going forward

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

Fixes #11276 

Though this change doesn't result in guaranteed Python 3.11 support, it should allow users of Python 3.11 to at least install Pulumi successfully without Deprecation warnings etc.  grpcio 1.4X does not ship 3.11-compatible wheels, so users are starting to see errors when installing Pulumi - bumping to this version also does not change our existing proto clients in Python, so feels like a relatively safe change.

## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


11437: Mark external modules as forign r=iwahbe a=iwahbe

Fixes #8070

Progress on #11427

Co-authored-by: Kyle Pitzen <kyle.pitzen@gmail.com>
Co-authored-by: Ian Wahbe <ian@wahbe.com>
bors bot added a commit that referenced this issue Nov 22, 2022
11431: Bumps grpcio to version 1.50 r=kpitzen a=kpitzen

Currently, pinning to a version of grpcio <1.50 can cause build errors on newer versions of python+pip - the existing setup.py install method of building libararies is being deprecated and wheels are not being backported for older versions of grpcio.  Since this change is a no-op as far as our python proto clients are concerned, it feels safe to bump to a version that will be supported going forward

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

Fixes #11276 

Though this change doesn't result in guaranteed Python 3.11 support, it should allow users of Python 3.11 to at least install Pulumi successfully without Deprecation warnings etc.  grpcio 1.4X does not ship 3.11-compatible wheels, so users are starting to see errors when installing Pulumi - bumping to this version also does not change our existing proto clients in Python, so feels like a relatively safe change.

## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


11437: Mark external modules as forign r=iwahbe a=iwahbe

Fixes #8070

Progress on #11427

Co-authored-by: Kyle Pitzen <kyle.pitzen@gmail.com>
Co-authored-by: Ian Wahbe <ian@wahbe.com>
@iwahbe
Copy link
Member

iwahbe commented Nov 23, 2022

Progress so far:

bors bot added a commit that referenced this issue Nov 23, 2022
11431: Bumps grpcio to version 1.50 r=kpitzen a=kpitzen

Currently, pinning to a version of grpcio <1.50 can cause build errors on newer versions of python+pip - the existing setup.py install method of building libararies is being deprecated and wheels are not being backported for older versions of grpcio.  Since this change is a no-op as far as our python proto clients are concerned, it feels safe to bump to a version that will be supported going forward

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

Fixes #11276 

Though this change doesn't result in guaranteed Python 3.11 support, it should allow users of Python 3.11 to at least install Pulumi successfully without Deprecation warnings etc.  grpcio 1.4X does not ship 3.11-compatible wheels, so users are starting to see errors when installing Pulumi - bumping to this version also does not change our existing proto clients in Python, so feels like a relatively safe change.

## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


11437: Mark external modules as forign r=justinvp a=iwahbe

Fixes #8070

Progress on #11427

11440: Cleanup import generation r=iwahbe a=iwahbe

Cleanup the `getPulumiImport` function.

In doing so, remove an edge causing extraneous `/index` in #11427. 


For reviewers, looking at the diff for this PR is painful. It's much easier to review the old and new code separately.

Part of #11427.

11441: Changelog and go.mod updates for v3.47.2 r=pulumi-bot a=pulumi-bot

bors merge

Co-authored-by: Kyle Pitzen <kyle.pitzen@gmail.com>
Co-authored-by: Ian Wahbe <ian@wahbe.com>
Co-authored-by: github-actions <github-actions@github.com>
@iwahbe
Copy link
Member

iwahbe commented Nov 23, 2022

Filed issue: pulumi/pulumi-aws-apigateway#64.
The following error is because pulumi-aws-apigateway sets the wrong languages.go.importBasePath.

go: finding module for package github.com/pulumi/pulumi-aws-apigateway/sdk/apigateway
demo2 imports
        github.com/pulumi/pulumi-aws-apigateway/sdk/apigateway: module github.com/pulumi/pulumi-aws-apigateway/sdk@latest found (v0.0.10), but does not contain package github.com/pulumi/pulumi-aws-apigateway/sdk/apigateway

It sets the base path as github.com/pulumi/pulumi-aws-apigateway/sdk/apigateway when it should set it as github.com/pulumi/pulumi-aws-apigateway/sdk/go/apigateway (notice the /go between the /sdk and /apigateway:

https://github.com/pulumi/pulumi-aws-apigateway/blob/d8a5e8d6264c51d0b979628e6b6d641bd1f0f97c/schema.yaml#L444

We can see a correct example in the pulumi-aws package:

https://github.com/pulumi/pulumi-aws/blob/b9f8fd1106181730aa644641950c1707525656bf/provider/cmd/pulumi-resource-aws/schema.json#L202

bors bot added a commit that referenced this issue Nov 23, 2022
11440: Cleanup import generation r=iwahbe a=iwahbe

Cleanup the `getPulumiImport` function.

In doing so, remove an edge causing extraneous `/index` in #11427. 


For reviewers, looking at the diff for this PR is painful. It's much easier to review the old and new code separately.

Part of #11427.

11441: Changelog and go.mod updates for v3.47.2 r=pulumi-bot a=pulumi-bot

bors merge

Co-authored-by: Ian Wahbe <ian@wahbe.com>
Co-authored-by: github-actions <github-actions@github.com>
bors bot added a commit that referenced this issue Nov 23, 2022
11431: Bumps grpcio to version 1.50 r=kpitzen a=kpitzen

Currently, pinning to a version of grpcio <1.50 can cause build errors on newer versions of python+pip - the existing setup.py install method of building libararies is being deprecated and wheels are not being backported for older versions of grpcio.  Since this change is a no-op as far as our python proto clients are concerned, it feels safe to bump to a version that will be supported going forward

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

Fixes #11276 

Though this change doesn't result in guaranteed Python 3.11 support, it should allow users of Python 3.11 to at least install Pulumi successfully without Deprecation warnings etc.  grpcio 1.4X does not ship 3.11-compatible wheels, so users are starting to see errors when installing Pulumi - bumping to this version also does not change our existing proto clients in Python, so feels like a relatively safe change.

## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


11437: Mark external modules as forign r=justinvp a=iwahbe

Fixes #8070

Progress on #11427

Co-authored-by: Kyle Pitzen <kyle.pitzen@gmail.com>
Co-authored-by: Ian Wahbe <ian@wahbe.com>
bors bot added a commit that referenced this issue Nov 23, 2022
11440: Cleanup import generation r=iwahbe a=iwahbe

Cleanup the `getPulumiImport` function.

In doing so, remove an edge causing extraneous `/index` in #11427. 


For reviewers, looking at the diff for this PR is painful. It's much easier to review the old and new code separately.

Part of #11427.

Co-authored-by: Ian Wahbe <ian@wahbe.com>
bors bot added a commit that referenced this issue Nov 23, 2022
11437: Mark external modules as forign r=justinvp a=iwahbe

Fixes #8070

Progress on #11427

Co-authored-by: Ian Wahbe <ian@wahbe.com>
bors bot added a commit that referenced this issue Nov 23, 2022
11440: Cleanup import generation r=iwahbe a=iwahbe

Cleanup the `getPulumiImport` function.

In doing so, remove an edge causing extraneous `/index` in #11427. 


For reviewers, looking at the diff for this PR is painful. It's much easier to review the old and new code separately.

Part of #11427.

Co-authored-by: Ian Wahbe <ian@wahbe.com>
bors bot added a commit that referenced this issue Nov 23, 2022
11440: Cleanup import generation r=iwahbe a=iwahbe

Cleanup the `getPulumiImport` function.

In doing so, remove an edge causing extraneous `/index` in #11427. 


For reviewers, looking at the diff for this PR is painful. It's much easier to review the old and new code separately.

Part of #11427.

Co-authored-by: Ian Wahbe <ian@wahbe.com>
bors bot added a commit that referenced this issue Nov 23, 2022
11442: Don't import types referenced from local variables r=iwahbe a=iwahbe

Don't import types if the type is only referenced in a scoped traversal (`foo.bar`), not named directly.

Part of #11427
Relates to #8324


Co-authored-by: Ian Wahbe <ian@wahbe.com>
@iwahbe iwahbe added the resolution/fixed This issue was fixed label Nov 30, 2022
@iwahbe
Copy link
Member

iwahbe commented Nov 30, 2022

As each of the underlying problems were fixed, we can now close the issue.

@iwahbe iwahbe closed this as completed Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/codegen SDK-gen, program-gen, convert kind/bug Some behavior is incorrect or out of spec language/go resolution/fixed This issue was fixed
Projects
None yet
Development

No branches or pull requests

2 participants