Skip to content
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

Build datastore index resource. #3085

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ def self_link_uri
if @self_link.nil?
[@base_url, '{{name}}'].join('/')
else
# If the terms in this are not snake-cased, this will require
# an override in Terraform.
@self_link
end
end
Expand Down
98 changes: 98 additions & 0 deletions products/datastore/api.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# Copyright 2020 Google Inc.
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

--- !ruby/object:Api::Product
name: Datastore
display_name: Cloud Datastore
versions:
- !ruby/object:Api::Product::Version
name: ga
base_url: https://datastore.googleapis.com/v1/
scopes:
- https://www.googleapis.com/auth/datastore
apis_required:
- !ruby/object:Api::Product::ApiReference
name: Cloud Datastore API
url: https://console.cloud.google.com/apis/library/datastore.googleapis.com
async: !ruby/object:Api::OpAsync
operation: !ruby/object:Api::OpAsync::Operation
path: 'name'
base_url: '{{op_id}}'
wait_ms: 1000
result: !ruby/object:Api::OpAsync::Result
path: 'response'
resource_inside_response: true
status: !ruby/object:Api::OpAsync::Status
path: 'done'
complete: true
allowed:
- true
- false
error: !ruby/object:Api::OpAsync::Error
path: 'error'
message: 'message'
objects:
- !ruby/object:Api::Resource
nat-henderson marked this conversation as resolved.
Show resolved Hide resolved
name: 'Index'
base_url: "projects/{{project}}/indexes"
self_link: "projects/{{project}}/indexes/{{indexId}}"
input: true
collection_url_key: indexes
references: !ruby/object:Api::Resource::ReferenceLinks
guides:
'Official Documentation': 'https://cloud.google.com/datastore/docs/concepts/indexes'
api: 'https://cloud.google.com/datastore/docs/reference/admin/rest/v1/projects.indexes'
identity:
nat-henderson marked this conversation as resolved.
Show resolved Hide resolved
- indexId
description: |
Describes a composite index for Cloud Datastore.
properties:
- !ruby/object:Api::Type::String
name: 'indexId'
output: true
description: |
The index id.
- !ruby/object:Api::Type::String
name: 'kind'
required: true
description: |
The entity kind which the index applies to.
- !ruby/object:Api::Type::Enum
name: 'ancestor'
default_value: :NONE
values:
- :NONE
- :ALL_ANCESTORS
description: |
Policy for including ancestors in the index. Either `ALL_ANCESTORS` or `NONE`,
the default is `NONE`.
- !ruby/object:Api::Type::Array
name: 'properties'
description: |
An ordered list of properties to index on.
min_size: 1
item_type: !ruby/object:Api::Type::NestedObject
properties:
- !ruby/object:Api::Type::String
name: 'name'
required: true
description: |
The property name to index.
- !ruby/object:Api::Type::Enum
name: 'direction'
required: true
values:
- :ASCENDING
- :DESCENDING
description: |
The direction the index should optimize for sorting. Possible values are ASCENDING and DESCENDING.
34 changes: 34 additions & 0 deletions products/datastore/terraform.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Copyright 2020 Google Inc.
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

--- !ruby/object:Provider::Terraform::Config
overrides: !ruby/object:Overrides::ResourceOverrides
Index: !ruby/object:Overrides::Terraform::ResourceOverride
id_format: "projects/{{project}}/indexes/{{index_id}}"
self_link: "projects/{{project}}/indexes/{{index_id}}"
autogen_async: true
# TODO(ndmckinley): This resource doesn't have a name, so the current
# sweeper won't ever sweep it - might as well not have one for now,
# but we'll add it in when we change the sweeper to account for
# identity.
skip_sweeper: true
timeouts: !ruby/object:Api::Timeouts
insert_minutes: 10
delete_minutes: 10
examples:
nat-henderson marked this conversation as resolved.
Show resolved Hide resolved
- !ruby/object:Provider::Terraform::Examples
name: "datastore_index"
primary_resource_id: "default"
vars:
property_name_1: "property_a"
property_name_2: "property_b"
11 changes: 11 additions & 0 deletions templates/terraform/examples/datastore_index.tf.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
resource "google_datastore_index" "<%= ctx[:primary_resource_id] %>" {
kind = "foo"
properties {
name = "<%= ctx[:vars]['property_name_1'] %>"
direction = "ASCENDING"
}
properties {
name = "<%= ctx[:vars]['property_name_2'] %>"
direction = "ASCENDING"
}
}
34 changes: 31 additions & 3 deletions templates/terraform/operation.go.erb
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ func (w *<%= product_name -%>OperationWaiter) QueryOp() (interface{}, error) {
return sendRequest(w.Config, "GET", <% if has_project %>w.Project<% else %>""<% end %>, url, nil<%= object.error_retry_predicates ? ", " + object.error_retry_predicates.join(',') : "" -%>)
}

func <%= product_name.camelize(:lower) -%>OperationWaitTime(config *Config, op map[string]interface{},<% if has_project -%> project,<% end -%> activity string, timeoutMinutes int) error {
func create<%= product_name %>Waiter(config *Config, op map[string]interface{}, <% if has_project -%> project, <% end -%> activity string) (*<%=product_name%>OperationWaiter, error) {
if val, ok := op["name"]; !ok || val == "" {
// This was a synchronous call - there is no operation to wait for.
return nil
return nil, nil
}
w := &<%= product_name -%>OperationWaiter{
Config: config,
Expand All @@ -38,7 +38,35 @@ func <%= product_name.camelize(:lower) -%>OperationWaitTime(config *Config, op m
<% end -%>
}
if err := w.CommonOperationWaiter.SetOp(op); err != nil {
return err
return nil, err
}
return w, nil
}

<% if async.result.resource_inside_response -%>
<%# Not all APIs will need a WithResponse operation, but it's hard to check whether
they will or not since it involves iterating over all resources.
Might as well just nolint it so we can pass the linter checks.
-%>
// nolint: deadcode,unused
func <%= product_name.camelize(:lower) -%>OperationWaitTimeWithResponse(config *Config, op map[string]interface{}, response *map[string]interface{},<% if has_project -%> project,<% end -%> activity string, timeoutMinutes int) error {
w, err := create<%= product_name %>Waiter(config, op, <% if has_project -%> project, <%end-%> activity)
if err != nil || w == nil {
// If w is nil, the op was synchronous.
return err
}
if err := OperationWait(w, activity, timeoutMinutes); err != nil {
return err
}
return json.Unmarshal([]byte(w.CommonOperationWaiter.Op.Response), response)
}
<% end -%>

func <%= product_name.camelize(:lower) -%>OperationWaitTime(config *Config, op map[string]interface{}, <% if has_project -%> project,<% end -%> activity string, timeoutMinutes int) error {
w, err := create<%= product_name %>Waiter(config, op, <% if has_project -%> project, <%end-%> activity)
if err != nil || w == nil {
// If w is nil, the op was synchronous.
return err
}
return OperationWait(w, activity, timeoutMinutes)
}
37 changes: 35 additions & 2 deletions templates/terraform/resource.erb
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,39 @@ func resource<%= resource_name -%>Create(d *schema.ResourceData, meta interface{
}
<% end -%>
<% if object.async.is_a? Api::OpAsync -%>
<% if object.async.result.resource_inside_response and not object.identity.empty? -%>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check- is the index id not known until the operation is finished, or is it also contained in the first operation that we then poll? I know firestore_index currently solves this with custom code in the post create, because the name is contained in the first operation response that we already have. Looking at the generated output for this we now set the id on that resource 3 times in create, which isn't ideal. Is there a clean solution that would work for that resource and this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right - it's not known until the operation is finished. :( I saw that it could be solved with custom code, but I wanted to make sure that I solved it Once And For All - is there an issue with setting the ID three times? I think we set it once to make sure that we'll do a refresh if we fail in the middle of polling, plus once to get the true final id once all information is known ... and then in firestore, once more due to the custom code solution. I agree it's not clean, and I'm struggling to test it so I don't even know if it works yet (since you can't use the same project for both firestore and datastore).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. I'm pretty ok with it, then. It shouldn't cause issues for users and especially with these templates going away at some point, I don't think it's worth spending a ton of time finding the most-clean-code way to solve this problem for two resources with similar but not identical needs.

var response map[string]interface{}
err = <%= client_name_camel -%>OperationWaitTimeWithResponse(
config, res, &response, <% if has_project -%> project, <% end -%> "Creating <%= object.name -%>",
int(d.Timeout(schema.TimeoutCreate).Minutes()))

if err != nil {
<% if object.custom_code.post_create_failure -%>
resource<%= resource_name -%>PostCreateFailure(d, meta)
<% end -%>
// The resource didn't actually create
d.SetId("")
return fmt.Errorf("Error waiting to create <%= object.name -%>: %s", err)
}
<% object.gettable_properties.each do |prop| -%>
<% if object.identity.include?(prop) -%>
if err := d.Set("<%= prop.name.underscore -%>", flatten<%= resource_name -%><%= titlelize_property(prop) -%>(response["<%= prop.api_name -%>"], d, config)); err != nil {
return err
}

<% end -%>
<% end -%>

// This may have caused the ID to update - update it if so.
id, err = replaceVars(d, config, "<%= id_format(object) -%>")
if err != nil {
return fmt.Errorf("Error constructing id: %s", err)
}
d.SetId(id)

<% else -%>
err = <%= client_name_camel -%>OperationWaitTime(
config, res,<% if has_project -%> project, <% end -%> "Creating <%= object.name -%>",
config, res, <% if has_project -%> project, <% end -%> "Creating <%= object.name -%>",
int(d.Timeout(schema.TimeoutCreate).Minutes()))

if err != nil {
Expand All @@ -246,7 +277,9 @@ func resource<%= resource_name -%>Create(d *schema.ResourceData, meta interface{
d.SetId("")
return fmt.Errorf("Error waiting to create <%= object.name -%>: %s", err)
}

<% end -%>
<% end -%>
<% end -%>

log.Printf("[DEBUG] Finished creating <%= object.name -%> %q: %#v", d.Id(), res)
Expand Down Expand Up @@ -652,14 +685,14 @@ func resource<%= resource_name -%>Delete(d *schema.ResourceData, meta interface{
}

<% if !object.async.nil? && object.async.allow?('delete') -%>

err = <%= client_name_camel -%>OperationWaitTime(
config, res, <% if has_project -%> project, <% end -%> "Deleting <%= object.name -%>",
int(d.Timeout(schema.TimeoutDelete).Minutes()))

if err != nil {
return err
}

<% end -%>

log.Printf("[DEBUG] Finished deleting <%= object.name -%> %q: %#v", d.Id(), res)
Expand Down
22 changes: 22 additions & 0 deletions third_party/terraform/utils/appengine_operation.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package google

import (
"encoding/json"
"fmt"
"regexp"

Expand Down Expand Up @@ -32,6 +33,27 @@ func appEngineOperationWait(config *Config, res interface{}, appId, activity str
return appEngineOperationWaitTime(config, res, appId, activity, 4)
}

func appEngineOperationWaitTimeWithResponse(config *Config, res interface{}, response *map[string]interface{}, appId, activity string, timeoutMinutes int) error {
op := &appengine.Operation{}
err := Convert(res, op)
if err != nil {
return err
}

w := &AppEngineOperationWaiter{
Service: config.clientAppEngine,
AppId: appId,
}

if err := w.SetOp(op); err != nil {
return err
}
if err := OperationWait(w, activity, timeoutMinutes); err != nil {
return err
}
return json.Unmarshal([]byte(w.CommonOperationWaiter.Op.Response), response)
}

func appEngineOperationWaitTime(config *Config, res interface{}, appId, activity string, timeoutMinutes int) error {
op := &appengine.Operation{}
err := Convert(res, op)
Expand Down
9 changes: 9 additions & 0 deletions third_party/terraform/website-compiled/google.erb
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,15 @@
</ul>
</li>

<li<%%= sidebar_current("docs-google-datastore") %>>
<a href="#">Google Datastore Resources</a>
<ul class="nav nav-visible">
<li<%%= sidebar_current("docs-google-datastore-index") %>>
<a href="/docs/providers/google/r/datastore_index.html">google_datastore_index</a>
</li>
</ul>
</li>

<li<%%= sidebar_current("docs-google-dataproc") %>>
<a href="#">Google Dataproc Resources</a>
<ul class="nav nav-visible">
Expand Down