Make not-generated Terraform resources CSV available as a Github workflow artifact #139
Conversation
ulucinar
force-pushed
the
fix-867
branch
6 times, most recently
from
9c944ff to
181cc42
Compare
cmd/generator/main.go
Outdated
| p := config.GetProvider() | ||
| pipeline.Run(p, absRootDir) | ||
| if fp := os.Getenv("SKIPPED_RESOURCES_CSV"); len(fp) != 0 { | ||
| if err := os.WriteFile(fp, []byte(strings.Join(p.GetSkippedResourceNames(), ",")), 0o600); err != nil { |
There was a problem hiding this comment.
should these be using ; so each resource is on its own row? looks like all the resources are generated into a single row now. Not sure which is better, it's just the first thing I noticed when opening the csv is that you have to scroll horizontally forever because they are in one row 😇
There was a problem hiding this comment.
Replaced the separator char with ;. @jbw976, which spreadsheet editor are you using? After seeing your comment, I gave it a try and it's opened by Numbers. With Numbers, both , and ; result in a table with a single row. If we would prefer a table with a single column, then we may use \n as the separator char here.
There was a problem hiding this comment.
yes sir, Pages gets opened by default on my Mac. When I mentioned ;, I didn't try it out or think it through :)
My personal opinion is that whatever works to make it a single column (e.g., \n) instead of single row is a more friendly scrolling experience. But that is not a super strong opinion and you can make the call here 🙇
There was a problem hiding this comment.
Then, let's proceed with a \n as the separator char to get a single column table.
…flow artifact Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
cmd/generator/main.go
Outdated
| pipeline.Run(config.GetProvider(), absRootDir) | ||
| p := config.GetProvider() | ||
| pipeline.Run(p, absRootDir) | ||
| if fp := os.Getenv("SKIPPED_RESOURCES_CSV"); len(fp) != 0 { |
There was a problem hiding this comment.
You may consider using a cli parsing library and accept both flags and env vars to keep argument parsing consistent like in uptest.
But it is totally up to you to deal with it now or later if we need more inputs. But at least a comment on why we use env here would be helpful.
There was a problem hiding this comment.
Done. Moved to the kingpin.v2 package for argument parsing in the generator.
…nerator Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
Description of your changes
This PR proposes a change where a CSV file of Terraform resource names which are available in the Terraform provider's schema but not generated as managed resources is produced and made available as a Github workflow artifact.
Currently, the artifact is generated when the
generatemake target is run as a dependency (ofcheck-diff). This is a little bit implicit and as an alternative we can generate the CSV on its own step. But I did not want to add to our build times (by runningmake generatea second time or computing the diff from the JSON schema and the generated files), and thus preferred to generate the CSV in theCheck Diffstep.I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested
Tested with the following workflow run:
https://github.com/upbound/provider-aws/actions/runs/3438916965