add iamcredentialsbasepath support for plugin framework #11947
Conversation
There was a problem hiding this comment.
Here's some initial feedback - I'll check the code locally on my next pass!
Currently there's an issue with generating the beta provider:
error parsing framework_config.go.tmpl for filepath /go/src/github.com/hashicorp/terraform-provider-google-beta/google-beta/fwtransport/framework_config.go template: framework_config.go.tmpl:526: function "version" not defined
mmv1/third_party/terraform/fwtransport/framework_config.go.tmpl
Outdated
Show resolved
Hide resolved
| @@ -520,6 +523,26 @@ func (p *FrameworkProviderConfig) SetupGrpcLogging() { | |||
| ) | |||
| } | |||
|
|
|||
| // Remove the `/{{version}}/` from a base path if present. | |||
| func RemoveBasePathVersion(url string) string { | |||
There was a problem hiding this comment.
Could we just import RemoveBasePathVersion from google/transport/config.go?
|
@BBBmau, this PR is waiting for action from you. If no action is taken, this PR will be closed in 28 days. Please address any comments or change requests, or re-request review from a core reviewer if no action is required. This notification can be disabled with the |
BBBmau
force-pushed
the
IAMCredentialsBasePath-pfconfig
branch
4 times, most recently
from
7e15243 to
8cb4e72
Compare
|
Latest changes allow you to build successfully on both ga and beta providers. The rebase from └─(09:25:40 on main ✹ ✭)──> devbuild 1 ↵ ──(Wed,Oct23)─┘
# github.com/hashicorp/terraform-provider-google/google/services/compute
google/services/compute/data_source_compute_secutity_policy.go:14:6: DataSourceGoogleComputeSecurityPolicy redeclared in this block
google/services/compute/data_source_compute_security_policy.go:14:6: other declaration of DataSourceGoogleComputeSecurityPolicyWasn't sure if to rebase on the feature-branch or not so i just decided to do it on here. Let me know if that's fine, i can also rebase on the feature branch itself. I also tried to manually test Unfortunately was running into the following error despite it being configured correctly. I can create the PR that includes the code that would add
|
|
resolved the previous comment. It was due to only running Currently running into issues where the My understanding is Is their anything else I'm not setting up correctly? @SarahFrench |
I think you should rebase onto the feature branch instead. If you have that "other declaration of DataSourceGoogleComputeSecurityPolicy" error locally it may be that you need to delete some files from the repo where you're generating the provider to. I had a similar issue at one point and it's because if you rename a file in your magic-modules repo and then generate the provider locally the destination directory that you're generating the provider to will have the old-named file still and then will have the new-named file also, causing errors like that. You'd need to delete the old-named file in that scenario, as the generation process doesn't delete files away. |
There was a problem hiding this comment.
Please rebase this PR so it's based on the feature branch.
The PR that caused the problem you're seeing was merged to main only 3 days ago, whereas the last sync into the feature branch included only commits from Sept 26th and earlier, so shouldn't affect us? I believe the problem is with your local environment as described here: #11947 (comment)
BBBmau
force-pushed
the
IAMCredentialsBasePath-pfconfig
branch
2 times, most recently
from
166e5c3 to
e7104a8
Compare
BBBmau
force-pushed
the
IAMCredentialsBasePath-pfconfig
branch
from
e7104a8 to
3b79b7f
Compare
rebased back to |
Tests analyticsTotal tests: 4100 Click here to see the affected service packages
Action takenFound 232 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
|
|
Thanks this makes review a lot easier! In future what you can do is checkout |
I'm confused why you'd see that problem, so maybe for now we should review this PR and then tackle any bugs in a subsequent PR where we have an ephemeral resource implemented + some acceptance tests. We should be able to write acc tests and run them locally now that there's an alpha TF binary with the necessary stuff. |
|
I used some of your reproduction info and I think there's a problem with provider configuration (i.e. the meta) being passed to the ephemeral resources, I'll follow up with you in Slack. I think that is a problem 'downstream' of this PR though so isn't a blocker for merging this |
BBBmau
merged commit df56ea7
into
GoogleCloudPlatform:FEATURE-BRANCH-ephemeral-resource
This is temporary and will be resolved once the muxing issues are addressed.
Release Note Template for Downstream PRs (will be copied)