-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow overriding the base path for a URL with custom endpoints #1880
Allow overriding the base path for a URL with custom endpoints #1880
Conversation
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesNo diff detected in Ansible. New Pull RequestsI built this PR into one or more new PRs on other repositories, and when those are closed, this PR will also be merged and closed. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
1 similar comment
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's worth using the endpoint naming schema, but I don't think we need to do it in most of our code- just what we expose to users. Do you think it would be worth having an endpoints block similar to what aws does?
Also, TGC has changes in just forwarding rule and target instance, I assume due to the custom expand changes. Is that ok?
@@ -164,6 +164,37 @@ func TestProvider_loadCredentialsFromJSON(t *testing.T) { | |||
} | |||
} | |||
|
|||
func TestAccProviderBasePath_setBasePath(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idea: right now, if the base path stuff completely did nothing, this test would still pass. What if, instead, you set the base path to something fake, and ensured that the resource didn't create?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sg- adding a negative test.
Using a block would be nice, but that prevents us from using environment variables to configure defaults unless the block has been defined. Based on my read of the equivalent Most of TGC uses an artificial path scheme (it's correct for newer resources, but different than real URIs for older ones), so I didn't touch that. After this change, they'll need to provide a provider config with base paths set to their default values in the validator library that depends on TGC. |
76968b9
to
0a9baf0
Compare
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
0a9baf0
to
b5104fc
Compare
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
@danawillow can you review for the changes between these commits? https://github.com/GoogleCloudPlatform/magic-modules/pull/1880/files/8701c8b78e280cc05d2ca3222202e2a4ba0feb3a..d53ba09c07526bc07d6b460306f811307bb5a327 edit: Looks like the sweeper uses a partial Config object, so I'll need to patch that. edit again: Actually nope, forgot some other stuff too. I'll add you back when I've fixed it. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
third_party/terraform/utils/provider_handwritten_endpoint.go.erb
Outdated
Show resolved
Hide resolved
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
ba506ac
to
b5d790e
Compare
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Tracked submodules are build/terraform-beta build/terraform-mapper build/terraform build/ansible build/inspec.
Add support for overriding the base path of the provider. Fixes hashicorp/terraform-provider-google#2962.
This is ready for review. It isn't ready to be merged because I've only added two products for now. They cover what needs to be done for every other product, so once this is approved I'll add the rest and then re-request review.
The test uses the beta endpoint instead of an emulator because it's hard to set up an emulator in an acc test framework. This tests that the feature works, and in the short-medium term we can get this set up to hit GCP staging APIs.
Also,
BasePath
naming comes from the client libs themselves.CustomEndpoint
is a bit closer to what user expectations are. Do you think it's worth changing to use that naming schema? For example, ingcloud
it'sgcloud config set api_endpoint_overrides/cloudfunctions http://localhost:8008/
to set an endpoint for Cloud Functions.Equivalent AWS provider feature: https://www.terraform.io/docs/providers/aws/guides/custom-service-endpoints.html