Presently, we implement code generation (e.g. for SDKs and
programs/`pulumi convert`) on a per-language basis in `pkg/codegen`.
Alongside these implementations, we have a set of tests built using a
common framework that use snapshots to verify that code generation
doesn't break when changes are made. Unfortunately, due to the way
things are currently laid out in our repository, these tests and their
dependencies are shipped as part of the `pkg/codegen` package. This
commit brings @blampe's work in
https://github.com/pulumi/pulumi/pull/16011 up to date and fixes this by
taking the following actions:
* Test harnesses that were previously located in
`pkg/codegen/<language>/test.go` are moved to
`pkg/codegen/testing/test/<language>.go`.
By default, Go excludes files ending in `_test.go`, but *not* files
named e.g. `test.go`. It might seem logical therefore to just rename
these files (e.g. to `codegen_test.go`), so that they can continue to
live alongside their language implementations. Unfortunately, while this
fixes one problem (dependencies pulling in test code), it introduces
another -- the existing `test.go` files actually exist to implement an
interface which is used by the common codegen-test framework we have.
Moving to `*_test.go` files would make them invisible to the actual
modules which run the tests through that framework.
* Test code is consequently refactored to clean up the separation of
test details (e.g. relevant fixtures) and execution implementation
(working directory, means of program generation).
* The `gen_program_test/generate.go` program, which we use to generate
the "batch tests", has been updated so that it respects the new code
layout and organisation.
As laid out by @blampe in #16011 (and shamelessly copied here), this
brings a number of benefits. Test dependencies are no longer included in
non-test packages that are consumed downstream. As an example, this
takes the `pulumi-language-go` binary from ~61MB down to ~36MB, and
speeds up build times:
```
go clean -cache && time go build .
(master) go build . 81.06s user 17.36s system 470% cpu 20.933 total
(PR) go build . 47.77s user 9.20s system 530% cpu 10.737 total
```
This doesn't completely remove these dependencies from downstream
because most also include `pkg/v3/testing/integration` in tests. It does
only compile these dependencies into the test binary, though.
Before:
```
❯ go mod why github.com/aws/aws-sdk-go-v2/service/sts
github.com/pulumi/pulumi-random/provider/v4/cmd/pulumi-tfgen-random
github.com/pulumi/pulumi-terraform-bridge/pf/tfgen
github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfgen
github.com/pulumi/pulumi/pkg/v3/codegen/dotnet
github.com/pulumi/pulumi/pkg/v3/testing/integration
github.com/pulumi/pulumi/pkg/v3/resource/stack
github.com/pulumi/pulumi/pkg/v3/secrets/cloud
github.com/pulumi/pulumi/pkg/v3/secrets/cloud.test
github.com/aws/aws-sdk-go-v2/service/sts
```
After:
```
github.com/pulumi/pulumi-random/provider/v4
github.com/pulumi/pulumi-random/provider/v4.test <- expected
github.com/pulumi/providertest
github.com/pulumi/pulumi/pkg/v3/testing/integration
github.com/pulumi/pulumi/pkg/v3/resource/stack
github.com/pulumi/pulumi/pkg/v3/secrets/cloud
github.com/pulumi/pulumi/pkg/v3/secrets/cloud.test
github.com/aws/aws-sdk-go-v2/service/sts
```
Note: this PR has been split into a number of commits, roughly one per
language, to make reviewing a bit easier. The commits *do not build
individually* due to the nature of the changes, but hopefully it makes
understanding the work a bit more feasible.
Closes#16011
---------
Co-authored-by: Bryce Lampe <bryce@pulumi.com>
This commit adds the `goheader` rule to `golangci-lint` to enforce that
all our Go source code includes appropriate licence headers, fixing up
files that currently fail that check.
---------
Co-authored-by: Will Jones <will@sacharissa.co.uk>
Enable the l1-empty test for Go.
This required some changes in reporting versions from the Go language
host (replaced dependencies don't really have a version anymore, because
they've been replaced by a local artefact and that doesn't contain any
version info itself). This then cascaded that the conformance test had
to be less strict about checking versions from GetDependencies because
Go now returns empty versions for these local deps.
<!---
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. -->
It appears that Go copies testdata into every GOMODCACHE of a project
that depends on pulumi/pkg; the schemas in codegen testdata add 300MB of
weight to the GOMODCACHE needed for download. What if we moved the
testdata out from under the tree.
The move looks like this:
```
from=pkg/codegen/testing/test/testdata
to=tests/testdata/codegen/
git mv "$from" "$to"
(cd pkg/codegen/testing/test && ln -s ../../../../tests/testdata/codegen ./testdata)
git add "$from"
```
The previous location is symlinked to the new location.
Evidence of `GOMODCACHE` pressure reduction:
https://gist.github.com/t0yv0/05dd8be5880171045aed01e123ae2b09
## Checklist
- [ ] I have run `make tidy` to update any new dependencies
- [ ] I have run `make lint` to verify my code passes the lint check
- [ ] I have formatted my code using `gofumpt`
<!--- 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.
-->
- [ ] 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 Cloud,
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
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
First step in addressing https://github.com/pulumi/pulumi/issues/14873
Note: I suspect I'll need to temporarily disable some codegen tests to
get this merged, and then once we release v3.98.0 and the next version
of esc, we can re-enable.
# Description
This PR fixes an issue in Go program where if users are writing invoke
expressions inline inside other expressions, then that invoke is
extracted into a temporary variable and then later referenced the same
way it was used. This is because non-output-versioned invokes cannot be
used directly since they return a tuple (InvokeResult, err) so we need
to check for the error before proceeding.
Fixes#14064
## Checklist
- [ ] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
- [x] I have formatted my code using `gofumpt`
<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [x] 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 Cloud,
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
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
This fixes how programgen generates import statements
to handle conflicting imports when two imported packages
have the same name, e.g.
github.com/pulumi/pulumi-aws/sdk/v5/go/aws/ecs
github.com/pulumi/pulumi-awsx/sdk/go/awsx/ecs
To do this, we add a fileImporter type that tracks these imports.
It prefers unnamed imports for packages unless one of the following is
true:
- the name of the package has already been used by another import
- the name of the package does not match the last component
of the import path (e.g., `example.com/foo-go` with `package foo`).
If the name has already been used by another import,
it attempts the following in-order:
- Combine the last two path components of the import path
into an identifier and use that if available.
e.g., `awsxs3` from `sdk/go/awsx/s3`.
- Append a number to the package name and increment it
until an unused name is found.
e.g. `ecs2`, `ecs3`, and so on.
There's a change in how this information is tracked as well.
Previously, this was a pull approach: various calls returned
programImports objects which all got merged together.
This change switches to a push approach:
as code is generated and imports are requested,
they're submitted to the fileImporter which keeps track of them
until the next `Reset()` call.
The above also has a nice side effect of dropping a parameter.
Another change worth explicitly calling out:
Previously, getModOrAlias partially duplicated some of the logic
implemented in getPulumiImport, and used `mod`, `originalMod`
in a non-obvious way.
This generated incorrect imports like the following
(note the two `/aws` at the end):
github.com/pulumi/pulumi-aws/sdk/v5/go/aws/aws
This change replicates more of the logic of getPulumiImport
(now called addPulumiImport) into this function,
and addresses the discrepancy in codegen caused by `mod`/`originalMod`.
The result leaves most existing code unchanged,
except in a couple existing cases where the resulting changes make sense
given the logic for named imports outlined above.
Resolves#11176
Per team discussion, switching to gofumpt.
[gofumpt][1] is an alternative, stricter alternative to gofmt.
It addresses other stylistic concerns that gofmt doesn't yet cover.
[1]: https://github.com/mvdan/gofumpt
See the full list of [Added rules][2], but it includes:
- Dropping empty lines around function bodies
- Dropping unnecessary variable grouping when there's only one variable
- Ensuring an empty line between multi-line functions
- simplification (`-s` in gofmt) is always enabled
- Ensuring multi-line function signatures end with
`) {` on a separate line.
[2]: https://github.com/mvdan/gofumpt#Added-rules
gofumpt is stricter, but there's no lock-in.
All gofumpt output is valid gofmt output,
so if we decide we don't like it, it's easy to switch back
without any code changes.
gofumpt support is built into the tooling we use for development
so this won't change development workflows.
- golangci-lint includes a gofumpt check (enabled in this PR)
- gopls, the LSP for Go, includes a gofumpt option
(see [installation instrutions][3])
[3]: https://github.com/mvdan/gofumpt#installation
This change was generated by running:
```bash
gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error)
```
The following files were manually tweaked afterwards:
- pkg/cmd/pulumi/stack_change_secrets_provider.go:
one of the lines overflowed and had comments in an inconvenient place
- pkg/cmd/pulumi/destroy.go:
`var x T = y` where `T` wasn't necessary
- pkg/cmd/pulumi/policy_new.go:
long line because of error message
- pkg/backend/snapshot_test.go:
long line trying to assign three variables in the same assignment
I have included mention of gofumpt in the CONTRIBUTING.md.
Stop using io/ioutil across the entire repository.
The io/ioutil package was deprecated in Go 1.16 (2021-02)
with replacements provided in other packages.
Specifically:
ioutil.Discard => io.Discard
ioutil.NopCloser => io.NopCloser
ioutil.ReadAll => io.ReadAll
ioutil.ReadFile => os.ReadFile
ioutil.TempDir => os.MkdirTemp
ioutil.TempFile => os.CreateTemp
ioutil.WriteFile => os.WriteFile
This change switches all of these entities
across the repository.
Following this change,
the only references to ioutil are in schema files:
% rg -l ioutil
pkg/codegen/testing/test/testdata/aws-4.26.0.json
pkg/codegen/testing/test/testdata/aws-4.36.0.json
pkg/codegen/testing/test/testdata/aws-4.37.1.json
pkg/codegen/testing/test/testdata/aws-5.4.0.json
pkg/codegen/testing/test/testdata/aws-5.16.2.json
The bulk of this change was generated automatically
with manual touch ups afterwards.
* Normalize providers merge
* Update CHANGELOG_PENDING.md
* Fix CHANGELOG_PENDING.md
* Add unit tests
* Ensure go lints
* Remove `Go Lint` as redundant
* Fix go.mod for sdk
* Fix some tests
* Correctly fetch the package name from a token
* Restore unnecessary changes
* Use the correct pkg for provider
As described in #8821, docs generated for helper functions can be incorrect because optional arguments to parameter objects are not correctly handled.
Previously, code would be generated like so:
```go
policyDocument, err := iam.GetPolicyDocument(ctx, &iam.GetPolicyDocumentArgs{
Statements: []iam.GetPolicyDocumentStatement{
iam.GetPolicyDocumentStatement{
Sid: "1",
//...
```
However "Sid" is of type `*string`.
This four helper conversion functions, to handle the primitive types we lower from, and modifies codegen to recursively apply these functions inside of an invoke call.
In the new code generation, the above is instead rendered as:
```go
policyDocument, err := iam.GetPolicyDocument(ctx, &iam.GetPolicyDocumentArgs{
Statements: []iam.GetPolicyDocumentStatement{
iam.GetPolicyDocumentStatement{
Sid: pulumi.StringRef("1"),
//...
```
* Move program tests into folders
* update package schema
* Enabled tests pass
* Fix lints and begin to update test cases
* Re-enable tests
* Update aws version to v4
* Refactor language specific parts
* Hook up dotnet and nodejs
* Update tests from master
* SSOT for schema/version
* Name blocking errors. Leave tests in valid state
* Give each language its own folder
* Remove SkipCompile for azure-sa (bug was fixed)
* Fix nits + changes asserts to require
* Remove unused import
* One last assert => require
These changes support arbitrary combinations of input + plain types
within a schema. Handling plain types at the property level was not
sufficient to support such combinations. Reifying these types
required updating quite a bit of code. This is likely to have caused
some temporary complications, but should eventually lead to
substantial simplification in the SDK and program code generators.
With the new design, input and optional types are explicit in the schema
type system. Optionals will only appear at the outermost level of a type
(i.e. Input<Optional<>>, Array<Optional<>>, etc. will not occur). In
addition to explicit input types, each object type now has a "plain"
shape and an "input" shape. The former uses only plain types; the latter
uses input shapes wherever a plain type is not specified. Plain types
are indicated in the schema by setting the "plain" property of a type spec
to true.