https://pulumi.com logo
#package-authoring
Title
# package-authoring
a

ancient-policeman-24615

12/28/2023, 1:58 PM
There need to be end-to-end tests in the bridge for
.MuxWith
(for sdk, pf and muxed providers). At minimum, we need to show that the new providers have their schemas added and that runtime dispatch works as expected. I think the clearest way to do that will be to write a test provider that uses
.MuxWith
. We have a couple of these test providers already in the bridge: https://github.com/pulumi/pulumi-terraform-bridge/blob/fe7b24d6a9128e06d8c281bd0f2f089676abadc7/internal/testprovider/schema_mini_random.go#L21-L23 https://github.com/pulumi/pulumi-terraform-bridge/blob/fe7b24d6a9128e06d8c281bd0f2f089676abadc7/pkg/tfgen/internal/testprovider/minirandom.go#L[…]28 For documentation, I meant that
muxer.Provider
and
tfbridge.ProviderInfo.MuxWith
need doc comments explaining what they do.
b

big-architect-71258

12/29/2023, 7:13 PM
@ancient-policeman-24615 @limited-rainbow-51650 opened a PR https://github.com/pulumi/pulumi-terraform-bridge/pull/1605 I was able to implement tests for the schema generation of a muxed provider. Not sure how to implement the validation that a muxed provider resource or function is correctly called. Will try to dig down in the code of the bridge to find a way how to do that. Of course I'll be happy about a hint!
@ancient-policeman-24615 I saw that PF in the Bridge uses a completely different approach to mux providers. (AugmentShimWithPF). The code is used at least in the Pulumi Cloudflare, Exoscale and MongoDBAtlas providers. So we can't introduce a breaking change here and align the code with what I created for the TF SDK V1 and V2 providers. Because we have no muxing for TF SDK providers yet we should align here with the code used for PF based providers. CC @limited-rainbow-51650 https://github.com/search?q=MuxShimWithPF&type=code
@ancient-policeman-24615 Main issue with this approach will be that the required interface
shim.Provider
does not align with the interface exposed by the GO Provider Framework
provider.Provider
. So you can't use the GO Provider Framework to create a mux provider. At least inconvenient. However, I would go so far as to say that all these interfaces for providers make the Pulumi ecosystem look chaotic in this area.
l

limited-rainbow-51650

12/30/2023, 3:03 PM
@big-architect-71258 I guess this is related: https://github.com/pulumi/pulumi-terraform-bridge/issues/1487
b

big-architect-71258

12/30/2023, 3:15 PM
@limited-rainbow-51650 not only the entry points got proliferated but as well the interfaces that you have to implement for a provider. This may sound egocentric, but compared to "my" code with a MuxWith array in the ProviderInfo struct, using the AugmentShimWithPF function and the necessary code to distinguish which provider (upstream or mux) is executing a resource or function is much more complex. @ancient-policeman-24615
a

ancient-policeman-24615

12/30/2023, 3:54 PM
I saw that PF in the Bridge uses a completely different approach to mux providers. (AugmentShimWithPF). The code is used at least in the Pulumi Cloudflare, Exoscale and MongoDBAtlas providers. So we can’t introduce a breaking change here and align the code with what I created for the TF SDK V1 and V2 providers. Because we have no muxing for TF SDK providers yet we should align here with the code used for PF based providers.
We will need to keep that functionality, since it is deployed in many of our providers. That said, I don’t think we need to align muxing a generic pulumi provider with the
AugmentShimWithPF
call. The two are meaningfully different. For example,
tfbridge.ProviderInfo
mappings apply to the SDK V{1,2} and PF providers. They will not apply to
MuxWith
addins. I think the
.MuxWith
design makes sense for adding in a generic provider.
Main issue with this approach will be that the required interface
shim.Provider
does not align with the interface exposed by the GO Provider Framework
provider.Provider
. So you can’t use the GO Provider Framework to create a mux provider. At least inconvenient. However, I would go so far as to say that all these interfaces for providers make the Pulumi ecosystem look chaotic in this area.
shim.Provider
is an abstraction over a TF provider.
provider.Provider
is an abstraction over a Pulumi provider. The interfaces are very different, but I agree that the work provider is overused here. --- @big-architect-71258 I’m reviewing your PR now.
b

big-architect-71258

12/30/2023, 6:34 PM
@ancient-policeman-24615 so are the two types of muxing disjunct? I.e. you can't (won't) use
AugmentShimWithPF
at the same time with
MuxWith
? Next possible issue:
pf
has a different entry point as
tfbridge
for SDK V{1,2} providers (as mentioned by @enough-garden-22763 in https://github.com/pulumi/pulumi-terraform-bridge/issues/1487). Currently I only added the code to evaluate
MuxWith
to
tfbridge
. Does the code in PF also need to be changed? And if so, how do we prevent a collision with
AugmentShimWithPF
?
@ancient-policeman-24615 pushed new commits according to your first review
a

ancient-policeman-24615

01/02/2024, 1:31 PM
@big-architect-71258 The two types of muxing can be disjoint in the initial PR, but long term they need to be composable. Right now, can you insert a check into
pf/tfbridge.serve
and
pf/tfbridge.MainWithMuxer
that errors if
len(ProviderInfo.MuxWith) > 0
. Eventually, we will want you to be able to combine arbitrary Terraform providers (served from
ProviderInfo.P
) and arbitrary Pulumi providers (served from
ProviderInfo.MuxWith
). I don’t think we need that all in 1 PR. A series of PRs is much more likely to merge then one monster PR.
I’m taking a look at the new changes now.
b

big-architect-71258

01/02/2024, 2:09 PM
@ancient-policeman-24615 Then I understood correctly that the PF implementation also needs to be changed and that one challenge is to combine
MuxWith
with
AugmentShimWithPF
. I totally agree with you that it is not a good idea to create a monster PR to squeeze the PF customizations into the current PR. However, I will add a
len(ProviderInfo.MuxWith) > 0
with an
error
to
pf/tfbridge.MainWithMuxer
to avoid problems with unadapted code there.
a

ancient-policeman-24615

01/02/2024, 2:12 PM
Then I understood correctly that the PF implementation also needs to be changed and that one challenge is to combine
MuxWith
with
AugmentShimWithPF
.
Yep
However, I will add a
len(ProviderInfo.MuxWith) > 0
with an
error
to
pf/tfbridge.MainWithMuxer
to avoid problems with unadapted code there.
Great
b

big-architect-71258

01/02/2024, 2:14 PM
@ancient-policeman-24615 There should be an issue for having the pf code changed for MuxWith. Shall I create one? Or is there already an issue that could be changed?
a

ancient-policeman-24615

01/02/2024, 2:14 PM
I don’t think there is an existing issue. Please feel free to create one.
b

big-architect-71258

01/02/2024, 2:15 PM
On my list 😁
@ancient-policeman-24615 pushed the code for
len(ProviderInfo.MuxWith) > 0
Had to use
panic
because the methods
MainWithMuxer
for
pf/tfgen
and
pf/tfbridge
do not return an
error
.
@ancient-policeman-24615 @enough-garden-22763 added tests for provider muxing. Issue right now:
liniting
fails. I local `make lint`is successful though.
image.png
Cleaning the local cache didn't make any difference.
l

limited-rainbow-51650

01/03/2024, 6:19 PM
@big-architect-71258 line 15: no go files to analyze. Are there any source files that weren't pushed to the branch perhaps?
b

big-architect-71258

01/03/2024, 6:21 PM
@limited-rainbow-51650 nope branch is clean. And I can't reproduce the error locally. Running the same command
make lint
and having the the same version of
golangci-lint
installed.
a

ancient-policeman-24615

01/03/2024, 6:25 PM
make lint
fails locally for me on your PR.
b

big-architect-71258

01/03/2024, 6:26 PM
@ancient-policeman-24615 WT*!?!?! 🙄 How could that be??
a

ancient-policeman-24615

01/03/2024, 6:26 PM
$ golangci-lint --version
golangci-lint has version 1.55.2 built with go1.21.3 from e3c2265 on 2023-11-02T214002Z
Could there be some ignored files that are linked?
git clean -fxd
is my go to hammer for this (but it will removed everything not checked into git).
b

big-architect-71258

01/03/2024, 6:28 PM
@ancient-policeman-24615 my version of
golangci-lint
is slightly different. Hard to spot the difference golangci-lint has version 1.55.2 built with go1.21.3 from e3c2265*f* on 2023-11-03T125925Z
Let me try the hammer 😄
a

ancient-policeman-24615

01/03/2024, 6:29 PM
@big-architect-71258 I think you are missing a
go mod tidy
in
pkg/tests
.
Running that got the linter working for me.
b

big-architect-71258

01/03/2024, 6:30 PM
@ancient-policeman-24615 after cleaning
golanglint-ci
fails.
l

limited-rainbow-51650

01/03/2024, 6:31 PM
Progress! Now it fails for everybody. 🤣
b

big-architect-71258

01/03/2024, 6:31 PM
After
go mod tidy
linting is successful. Pushing the changes now.
@ancient-policeman-24615 Thanks for helping!!
a

ancient-policeman-24615

01/03/2024, 6:32 PM
No problem.
b

big-architect-71258

01/03/2024, 6:33 PM
Something learned again. 👍🏻 But I've to admin the error message of
golanglint-ci
could be improved. Changes pushed BTW
@ancient-policeman-24615 @enough-garden-22763 do we need another review for the PR, or could it be merged now?
e

enough-garden-22763

01/03/2024, 7:56 PM
Yeah I'll review again tomorrow-ish. Thank you!