Generalize bridged template for reuse with NoUpstream providers #1278
Conversation
t0yv0
force-pushed
the
t0yv0/generalize-bridged-template
branch
from
e265b9d to
8148800
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
For NoUpstream providers make .make/upstream is a no-op but it still needs to succeed as a real target and touch the sentinel file.
t0yv0
force-pushed
the
t0yv0/generalize-bridged-template
branch
from
8148800 to
22b93b4
Compare
This comment has been minimized.
This comment has been minimized.
t0yv0
changed the title
| @@ -1,3 +1,4 @@ | |||
| #{{ if not .Config.NoUpstream -}}# | |||
There was a problem hiding this comment.
What does GitHub do if it encounters an empty yaml file in the workflows directory? Does that work?
There was a problem hiding this comment.
Looks like the empty files just don't get generated (see https://github.com/pulumi/pulumi-awsx/pull/1471/files )
There was a problem hiding this comment.
GitHub actions will skip/ignore the empty yaml file
| @@ -287,7 +287,11 @@ tfgen_build_only: bin/$(TFGEN) | |||
| bin/$(TFGEN): provider/*.go provider/go.* .make/upstream | |||
| (cd provider && go build $(PULUMI_PROVIDER_BUILD_PARALLELISM) -o $(WORKING_DIR)/bin/$(TFGEN) -ldflags "$(LDFLAGS_PROJ_VERSION) $(LDFLAGS_EXTRAS)" $(PROJECT)/$(PROVIDER_PATH)/cmd/$(TFGEN)) | |||
| .PHONY: tfgen schema tfgen_no_deps tfgen_build_only | |||
|
|
|||
| #{{ if .Config.NoUpstream }}# | |||
There was a problem hiding this comment.
Shouldn't this test for .Config.NoUpstream being set to false?
There was a problem hiding this comment.
I think the code is correct. It's an if-then-else. If NoUpstream, we generate a dummy .make/upstream target. That is still required sadly. If NoUpstream=false, we hit the else branch which generates a bridge-specific upstream target.
There was a problem hiding this comment.
No. I believe it should be NoUpsteam == true, as this branch is just creating a dummy upstream folder instead of actually populating the upstream submodule folder.
There was a problem hiding this comment.
So code is correct yes? here's how it renders for AWS (
) - no changes, which is good. The new form renders for eks.There was a problem hiding this comment.
@t0yv0 Yes, the code is correct. My message was sent around the same time as your reply 😄
There was a problem hiding this comment.
Ah, I got confused by adding an upstream make target in this case. Might be worth adding a comment to explain that this is a dummy make target
| @@ -265,6 +265,10 @@ type Config struct { | |||
| // MakeTemplate has no effect but is set by 78 providers. | |||
| // https://github.com/search?q=org%3Apulumi+path%3A.ci-mgmt.yaml+%22makeTemplate%3A%22&type=code | |||
| MakeTemplate string `yaml:"makeTemplate"` | |||
|
|
|||
| // NoUpstream is a temporary hack to disable bridge-specific workflow steps | |||
There was a problem hiding this comment.
What is temporary about it? What do we plan on doing long term?
There was a problem hiding this comment.
Yeah I'm taking this comment from Bryce's original PR. It might have been a part of a larger plan.
Co-authored-by: Florian Stadler <[email protected]>
|
🚀 The Update (preview) for pulumi/pulumi-provider-repos/production was successful. Resource Changes Name Type Operation
+ pulumi-eks-awaiting/bridge github:index/issueLabel:IssueLabel create
+ pulumi-eks-awaiting/core github:index/issueLabel:IssueLabel create
+ pulumi-eks-needs-release/minor github:index/issueLabel:IssueLabel create
+ pulumi-eks-needs-release/patch github:index/issueLabel:IssueLabel create
+ eks-default github:index/branchProtection:BranchProtection create
+ pulumi-eks-needs-release/major github:index/issueLabel:IssueLabel create
+ eks pkg:provider:Labels create
+ pulumi-eks-awaiting/codegen github:index/issueLabel:IssueLabel create
... and 55 other changes |
In #1278 we added pulumi-eks as a test provider which is good but we also added it to providers.json - it is not quite ready for this since the repo has not been fully onboarded. One observable failure is that ci-mgmt started applying branch protections to the repository and failed to override hand-rolled branch protections. It is better to avoid doing this for now.
This is #1268 without the sharding. This should be safe as it is not affecting any providers per se yet, just building up a capability.