Skip to content

Commit

Permalink
Allow overriding the base path for a URL with custom endpoints (#1880)
Browse files Browse the repository at this point in the history
Merged PR #1880.
  • Loading branch information
rileykarson authored and modular-magician committed Jun 13, 2019
1 parent e5120cd commit caf4024
Show file tree
Hide file tree
Showing 29 changed files with 893 additions and 130 deletions.
64 changes: 36 additions & 28 deletions api/resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -305,57 +305,65 @@ def exclude_if_not_in_version!(version)
# In newer resources there is much less standardisation in terms of value.
# Generally for them though, it's the product.base_url + resource.name
def self_link_url
base_url = @__product.base_url
[@__product.base_url, self_link_uri].flatten.join
end

# Returns the partial uri / relative path of a resource. In newer resources,
# this is the name. This fn is named self_link_uri for consistency, but
# could otherwise be considered to be "path"
def self_link_uri
if @self_link.nil?
[base_url, [@base_url, '{{name}}'].join('/')].flatten.join
[@base_url, '{{name}}'].join('/')
else
self_link = @self_link
[base_url, self_link].flatten.join
@self_link
end
end

def collection_url
[
@__product.base_url,
@base_url
].flatten.join
[@__product.base_url, collection_uri].flatten.join
end

def collection_uri
@base_url
end

def async_operation_url
[@__product.base_url, async_operation_uri].flatten.join
end

def async_operation_uri
raise 'Not an async resource' if async.nil?

[@__product.base_url, async.operation.base_url].flatten.join
async.operation.base_url
end

def default_create_url
if @create_verb.nil? || @create_verb == :POST
collection_url
elsif @create_verb == :PUT
self_link_url
else
raise "unsupported create verb #{@create_verb}"
end
def full_create_url
[@__product.base_url, create_uri].flatten.join
end

def full_create_url
def create_uri
if @create_url.nil?
default_create_url
if @create_verb.nil? || @create_verb == :POST
collection_uri
elsif @create_verb == :PUT
self_link_uri
else
raise "unsupported create verb #{@create_verb}"
end
else
[
@__product.base_url,
@create_url
].flatten.join
@create_url
end
end

def full_delete_url
[@__product.base_url, delete_uri].flatten.join
end

def delete_uri
if @delete_url.nil?
self_link_url
self_link_uri
else
[
@__product.base_url,
@delete_url
].flatten.join
@delete_url
end
end

Expand Down
2 changes: 1 addition & 1 deletion build/terraform
2 changes: 1 addition & 1 deletion build/terraform-beta
8 changes: 6 additions & 2 deletions provider/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,13 @@ def custom_update_properties_by_url(properties, update_url)
end

def update_url(resource, url_part)
return resource.self_link_url if url_part.nil?
[resource.__product.base_url, update_uri(resource, url_part)].flatten.join
end

def update_uri(resource, url_part)
return resource.self_link_uri if url_part.nil?

[resource.__product.base_url, url_part].flatten.join
url_part
end

# TODO(nelsonjr): Review all object interfaces and move to private methods
Expand Down
4 changes: 2 additions & 2 deletions templates/terraform/custom_expand/self_link_from_name.erb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func expand<%= prefix -%><%= titlelize_property(property) -%>(v interface{}, d T
return "https://www.googleapis.com/compute/v1/" + v.(string), nil
} else if strings.HasPrefix(v.(string), "regions/") || strings.HasPrefix(v.(string), "zones/") {
// For regional or zonal resources which include their region or zone, just put the project in front.
url, err := replaceVars(d, config, "https://www.googleapis.com/compute/v1/projects/{{project}}/")
url, err := replaceVars(d, config, "{{ComputeBasePath}}projects/{{project}}/")
if err != nil {
return nil, err
}
Expand All @@ -34,7 +34,7 @@ func expand<%= prefix -%><%= titlelize_property(property) -%>(v interface{}, d T
// Anything else is assumed to be a regional resource, with a partial link that begins with the resource name.
// This isn't very likely - it's a last-ditch effort to extract something useful here. We can do a better job
// as soon as MultiResourceRefs are working since we'll know the types that this field is supposed to point to.
url, err := replaceVars(d, config, "https://www.googleapis.com/compute/v1/projects/{{project}}/regions/{{region}}/")
url, err := replaceVars(d, config, "{{ComputeBasePath}}projects/{{project}}/regions/{{region}}/")
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion templates/terraform/examples/base_configs/test_file.go.erb
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func testAccCheck<%= resource_name -%>Destroy(s *terraform.State) error {

config := testAccProvider.Meta().(*Config)

url, err := replaceVarsForTest(rs, "<%= object.self_link_url -%>")
url, err := replaceVarsForTest(rs, "<%= "{{#{object.__product.name}BasePath}}#{object.self_link_uri}" -%>")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion templates/terraform/post_create/labels.erb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ if v, ok := d.GetOkExists("labels"); !isEmptyValue(reflect.ValueOf(v)) && (ok ||
labelFingerprintProp := d.Get("label_fingerprint")
obj["labelFingerprint"] = labelFingerprintProp

url, err = replaceVars(d, config, "<%= object.self_link_url -%>/setLabels")
url, err = replaceVars(d, config, "<%= "{{#{object.__product.name}BasePath}}#{object.self_link_uri}" -%>/setLabels")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion templates/terraform/pre_delete/detach_network.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ if d.Get("networks.#").(int) > 0 {
patched := make(map[string]interface{})
patched["networks"] = nil

url, err := replaceVars(d, config, "https://www.googleapis.com/dns/v1beta2/projects/{{project}}/policies/{{name}}")
url, err := replaceVars(d, config, "{{DnsBasePath}}projects/{{project}}/policies/{{name}}")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion templates/terraform/pre_delete/modify_delete_url.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// in theory, we should find a way to disable the default URL and not construct
// both, but that's a problem for another day. Today, we cheat.
log.Printf("[DEBUG] replacing URL %q with a custom delete URL", url)
url, err = replaceVars(d, config, "<%= object.__product.base_url -%><%=object.base_url-%>/{{name}}")
url, err = replaceVars(d, config, "<%= "{{#{object.__product.name}BasePath}}" -%><%=object.base_url-%>/{{name}}")
if err != nil {
return err
}
13 changes: 13 additions & 0 deletions templates/terraform/provider_gen.erb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,19 @@ package google

import "github.com/hashicorp/terraform/helper/schema"

// If the base path has changed as a result of your PR, make sure to update
// the provider_reference page!
var <%= product_ns -%>DefaultBasePath = "<%= product.base_url -%>"
var <%= product_ns -%>CustomEndpointEntryKey = "<%= product_ns.underscore -%>_custom_endpoint"
var <%= product_ns -%>CustomEndpointEntry = &schema.Schema{
Type: schema.TypeString,
Optional: true,
ValidateFunc: validateCustomEndpoint,
DefaultFunc: schema.MultiEnvDefaultFunc([]string{
"GOOGLE_<%= product_ns.underscore.upcase -%>_CUSTOM_ENDPOINT",
}, <%= product_ns -%>DefaultBasePath),
}

var Generated<%= product_ns -%>ResourcesMap = map[string]*schema.Resource{
<% product.objects.reject { |r| r.exclude || r.not_in_version?(product.version_obj_or_default(version)) }.each do |object| -%>
<%
Expand Down
10 changes: 5 additions & 5 deletions templates/terraform/resource.erb
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func resource<%= resource_name -%>Create(d *schema.ResourceData, meta interface{
defer mutexKV.Unlock(lockName)
<% end -%>

url, err := replaceVars(d, config, "<%= object.full_create_url -%>")
url, err := replaceVars(d, config, "<%= "{{#{object.__product.name}BasePath}}#{object.create_uri}" -%>")
if err != nil {
return err
}
Expand Down Expand Up @@ -199,7 +199,7 @@ func resource<%= resource_name -%>Create(d *schema.ResourceData, meta interface{
func resource<%= resource_name -%>Read(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)

url, err := replaceVars(d, config, "<%= object.self_link_url -%>")
url, err := replaceVars(d, config, "<%= "{{#{object.__product.name}BasePath}}#{object.self_link_uri}" -%>")
if err != nil {
return err
}
Expand Down Expand Up @@ -327,7 +327,7 @@ if <%= props.map { |prop| "d.HasChange(\"#{prop.name.underscore}\")" }.join ' ||
defer mutexKV.Unlock(lockName)
<% end -%>

url, err := replaceVars(d, config, "<%= update_url(object, key[:update_url]) -%>")
url, err := replaceVars(d, config, "<%= "{{#{object.__product.name}BasePath}}#{update_uri(object, key[:update_url])}" -%>")
if err != nil {
return err
}
Expand Down Expand Up @@ -417,7 +417,7 @@ if <%= props.map { |prop| "d.HasChange(\"#{prop.name.underscore}\")" }.join ' ||
defer mutexKV.Unlock(lockName)
<% end -%>

url, err := replaceVars(d, config, "<%= update_url(object, object.update_url) -%>")
url, err := replaceVars(d, config, "<%= "{{#{object.__product.name}BasePath}}#{update_uri(object, object.update_url)}" -%>")
if err != nil {
return err
}
Expand Down Expand Up @@ -485,7 +485,7 @@ func resource<%= resource_name -%>Delete(d *schema.ResourceData, meta interface{
defer mutexKV.Unlock(lockName)
<% end -%>

url, err := replaceVars(d, config, "<%= object.full_delete_url -%>")
url, err := replaceVars(d, config, "<%= "{{#{object.__product.name}BasePath}}#{object.delete_uri}" -%>")
if err != nil {
return err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func dataSourceGoogleComposerImageVersionsRead(d *schema.ResourceData, meta inte
return err
}

url, err := replaceVars(d, config, "https://composer.googleapis.com/v1/projects/{{project}}/locations/{{region}}/imageVersions")
url, err := replaceVars(d, config, "{{ComposerBasePath}}projects/{{project}}/locations/{{region}}/imageVersions")
if err != nil {
return err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func dataSourceTpuTensorFlowVersionsRead(d *schema.ResourceData, meta interface{
return err
}

url, err := replaceVars(d, config, "https://tpu.googleapis.com/v1/projects/{{project}}/locations/{{zone}}/tensorflowVersions")
url, err := replaceVars(d, config, "{{TpuBasePath}}projects/{{project}}/locations/{{zone}}/tensorflowVersions")
if err != nil {
return err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func testAccCheckAccessContextManagerAccessLevelDestroy(s *terraform.State) erro

config := testAccProvider.Meta().(*Config)

url, err := replaceVarsForTest(rs, "https://accesscontextmanager.googleapis.com/v1beta/{{name}}")
url, err := replaceVarsForTest(rs, "{{AccessContextManagerBasePath}}{{name}}")
if err != nil {
return err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func testAccCheckAccessContextManagerAccessPolicyDestroy(s *terraform.State) err

config := testAccProvider.Meta().(*Config)

url, err := replaceVarsForTest(rs, "https://accesscontextmanager.googleapis.com/v1beta/accessPolicies/{{name}}")
url, err := replaceVarsForTest(rs, "{{AccessContextManagerBasePath}}accessPolicies/{{name}}")
if err != nil {
return err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func testAccCheckAccessContextManagerServicePerimeterDestroy(s *terraform.State)

config := testAccProvider.Meta().(*Config)

url, err := replaceVarsForTest(rs, "https://accesscontextmanager.googleapis.com/v1beta/{{name}}")
url, err := replaceVarsForTest(rs, "{{AccessContextManagerBasePath}}{{name}}")
if err != nil {
return err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func checkComputeBackendBucketSignedUrlKeyExists(s *terraform.State) (bool, erro
config := testAccProvider.Meta().(*Config)
keyName := rs.Primary.ID

url, err := replaceVarsForTest(rs, "https://www.googleapis.com/compute/v1/projects/{{project}}/global/backendBuckets/{{backend_bucket}}")
url, err := replaceVarsForTest(rs, "{{ComputeBasePath}}projects/{{project}}/global/backendBuckets/{{backend_bucket}}")
if err != nil {
return false, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func checkComputeBackendServiceSignedUrlKeyExists(s *terraform.State) (bool, err
config := testAccProvider.Meta().(*Config)
keyName := rs.Primary.ID

url, err := replaceVarsForTest(rs, "https://www.googleapis.com/compute/v1/projects/{{project}}/global/backendServices/{{backend_service}}")
url, err := replaceVarsForTest(rs, "{{ComputeBasePath}}projects/{{project}}/global/backendServices/{{backend_service}}")
if err != nil {
return false, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,9 @@ func getInitializedConfig(t *testing.T) *Config {
Region: getTestRegionFromEnv(),
Zone: getTestZoneFromEnv(),
}

ConfigureBasePaths(config)

err := config.LoadAndValidate()
if err != nil {
t.Fatal(err)
Expand Down
12 changes: 8 additions & 4 deletions third_party/terraform/utils/bootstrap_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,15 @@ func BootstrapKMSKeyInLocation(t *testing.T, locationID string) bootstrappedKMS
keyParent := fmt.Sprintf("projects/%s/locations/%s/keyRings/%s", projectID, locationID, SharedKeyRing)
keyName := fmt.Sprintf("%s/cryptoKeys/%s", keyParent, SharedCyptoKey)

config := Config{
config := &Config{
Credentials: getTestCredsFromEnv(),
Project: getTestProjectFromEnv(),
Region: getTestRegionFromEnv(),
Zone: getTestZoneFromEnv(),
}

ConfigureBasePaths(config)

if err := config.LoadAndValidate(); err != nil {
t.Errorf("Unable to bootstrap KMS key: %s", err)
}
Expand Down Expand Up @@ -116,7 +118,7 @@ var serviceAccountDisplay = "Bootstrapped Service Account for Terraform tests"
// Some tests need a second service account, other than the test runner, to assert functionality on.
// This provides a well-known service account that can be used when dynamically creating a service
// account isn't an option.
func getOrCreateServiceAccount(config Config, project string) (*iam.ServiceAccount, error) {
func getOrCreateServiceAccount(config *Config, project string) (*iam.ServiceAccount, error) {
name := fmt.Sprintf("projects/%s/serviceAccounts/%s@%s.iam.gserviceaccount.com", project, serviceAccountEmail, project)
log.Printf("[DEBUG] Verifying %s as bootstrapped service account.\n", name)

Expand Down Expand Up @@ -148,7 +150,7 @@ func getOrCreateServiceAccount(config Config, project string) (*iam.ServiceAccou
// on a different service account. Granting permissions takes time and there is no operation to wait on
// so instead this creates a single service account once per test-suite with the correct permissions.
// The first time this test is run it will fail, but subsequent runs will succeed.
func impersonationServiceAccountPermissions(config Config, sa *iam.ServiceAccount, testRunner string) error {
func impersonationServiceAccountPermissions(config *Config, sa *iam.ServiceAccount, testRunner string) error {
log.Printf("[DEBUG] Setting service account permissions.\n")
policy := iam.Policy{
Bindings: []*iam.Binding{},
Expand Down Expand Up @@ -179,13 +181,15 @@ func BootstrapServiceAccount(t *testing.T, project, testRunner string) string {
return ""
}

config := Config{
config := &Config{
Credentials: getTestCredsFromEnv(),
Project: getTestProjectFromEnv(),
Region: getTestRegionFromEnv(),
Zone: getTestZoneFromEnv(),
}

ConfigureBasePaths(config)

if err := config.LoadAndValidate(); err != nil {
t.Fatalf("Bootstrapping failed. Unable to load test config: %s", err)
}
Expand Down
Loading

0 comments on commit caf4024

Please sign in to comment.