Skip to content

Commit

Permalink
Merging readonly + virtual (#333)
Browse files Browse the repository at this point in the history
Merged PR #333.
  • Loading branch information
rambleraptor authored and modular-magician committed Jul 10, 2018
1 parent 1d19fb9 commit bdc8ab7
Show file tree
Hide file tree
Showing 25 changed files with 81 additions and 83 deletions.
24 changes: 12 additions & 12 deletions GOVERNANCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -287,15 +287,15 @@ to remind you where they belong improves readability

<% objects.each do |obj| -%>
...
<% if obj.virtual -%>
<% if obj.readonly -%>
...
... many lines later
...
<% elsif !obj.virtual && obj.broken -%>
<% elsif !obj.readonly && obj.broken -%>
...
... many lines later
...
<% end # if obj.virtual -%>
<% end # if obj.readonly -%>
...
... many lines later
...
Expand All @@ -311,14 +311,14 @@ the example below it is easier to note the if is inside the `each` and there are

<% objects.each do |obj| -%>
...
<% if obj.virtual -%>
<% if obj.readonly -%>
...
<% if obj.input -%>
...
... many lines later
...
<% end # if obj.input -%>
<% end # if obj.virtual -%>
<% end # if obj.readonly -%>
...
... many lines later
...
Expand All @@ -328,14 +328,14 @@ the example below it is easier to note the if is inside the `each` and there are

<% objects.each do |obj| -%>
...
<% if obj.virtual -%>
<% if obj.readonly -%>
...
<% if obj.input -%>
...
... many lines later
...
<% end # if obj.input -%>
<% end # if obj.virtual -%>
<% end # if obj.readonly -%>
...
... many lines later
...
Expand Down Expand Up @@ -417,8 +417,8 @@ phases.
**Good**

<%=
# List all virtual properties that are not nested objects alphabetically
lines(object.all_properties.select(&:virtual)
# List all readonly properties that are not nested objects alphabetically
lines(object.all_properties.select(&:readonly)
.reject { |p| p.is_a?(Api::Type::NestedObject) }
.sort
.map { |p| "- #{p.out_name}" })
Expand All @@ -427,12 +427,12 @@ phases.
**Bad**

<%
# List all virtual properties that are not nested objects alphabetically
# List all readonly properties that are not nested objects alphabetically
my_properties = []
object.all_proerties.each do |p|
if p.virtual && !p.is_a?(Api::Type::NestedObject)
if p.readonly && !p.is_a?(Api::Type::NestedObject)
my_properties << p
end # p.virtual
end # p.readonly
end # object...each
%>
<% my_properties.sort.each do |p| -%>
Expand Down
2 changes: 0 additions & 2 deletions api/resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ module Properties
# If empty, we assume that `name` is the identifier.
attr_reader :identity
attr_reader :exclude
attr_reader :virtual
attr_reader :async
attr_reader :readonly
attr_reader :exports
Expand Down Expand Up @@ -235,7 +234,6 @@ def validate
check_optional_property :exports, Array
check_optional_property :self_link, String
check_optional_property :self_link_query, Api::Resource::ResponseList
check_optional_property :virtual, :boolean
check_optional_property :readonly, :boolean
check_optional_property :label_override, String
check_optional_property :transport, Transport
Expand Down
2 changes: 1 addition & 1 deletion build/chef/compute
2 changes: 1 addition & 1 deletion build/chef/sql
14 changes: 7 additions & 7 deletions products/_bundle/templates/ansible/lookup.erb
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ options:
default: null
choices:
<%
# Looking for virtual objects that export for ResourceRefs
virtuals = products.map { |product, _| product.objects }
# Looking for readonly objects that export for ResourceRefs
readonlys = products.map { |product, _| product.objects }
.flatten
.select(&:virtual)
.select(&:readonly)
.reject { |obj| obj.exports.nil? }
-%>
<% virtuals.each do |virt| -%>
<% readonlys.each do |virt| -%>
- <%= Google::StringUtils.underscore(virt.name) %>
<% end -%>
return:
Expand Down Expand Up @@ -95,7 +95,7 @@ class GcpModule(object):
raise AnsibleError(kwargs['msg'])


<% virtuals.each do |object| -%>
<% readonlys.each do |object| -%>
<%
prod_name = object.__product.prefix[1..-1]
-%>
Expand Down Expand Up @@ -153,14 +153,14 @@ class Gcp<%= object.name -%>(object):
return response[<%= quote_string(default) -%>]


<% end # virtuals.each -%>
<% end # readonlys.each -%>
class LookupModule(LookupBase):
def run(self, terms, variables, **kwargs):

self.set_options(var_options=variables, direct=kwargs)
<%
opts_code = []
virtuals.each do |object|
readonlys.each do |object|
name = Google::StringUtils.underscore(object.name)
opts_code << "#{quote_string(name)}: Gcp#{object.name}"
end
Expand Down
10 changes: 5 additions & 5 deletions products/compute/api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ objects:
# triggered for create. Implement support for read only objects, and delete
# the virtual tag
# | readonly: true
virtual: true
readonly: true
exports:
- !ruby/object:Api::Type::SelfLink
name: 'selfLink'
Expand Down Expand Up @@ -1516,7 +1516,7 @@ objects:
name: 'License'
kind: 'compute#license'
base_url: /projects/{{project}}/global/licenses
virtual: true
readonly: true
exports:
- !ruby/object:Api::Type::SelfLink
name: 'selfLink'
Expand Down Expand Up @@ -2145,7 +2145,7 @@ objects:
- 'name'
- !ruby/object:Api::Type::SelfLink
name: 'selfLink'
virtual: true
readonly: true
description: |
Represents a MachineType resource. Machine types determine the virtualized
hardware specifications of your virtual machine instances, such as the
Expand Down Expand Up @@ -2334,7 +2334,7 @@ objects:
- name
- !ruby/object:Api::Type::SelfLink
name: 'selfLink'
virtual: true
readonly: true
description: |
Represents a Region resource. A region is a specific geographical
location where you can run your resources. Each region has one or more
Expand Down Expand Up @@ -3628,7 +3628,7 @@ objects:
- name
- !ruby/object:Api::Type::SelfLink
name: 'selfLink'
virtual: true
readonly: true
description: 'Represents a Zone resource.'
properties:
- !ruby/object:Api::Type::Time
Expand Down
2 changes: 1 addition & 1 deletion products/dns/api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ objects:
A project resource. The project is a top level container for resources
including Cloud DNS ManagedZones.
base_url: 'projects'
virtual: true
readonly: true
properties:
- !ruby/object:Api::Type::Integer
name: 'number'
Expand Down
2 changes: 1 addition & 1 deletion products/spanner/api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ objects:
exports:
- !ruby/object:Api::Type::FetchedExternal
name: name
virtual: true
readonly: true
transport: !ruby/object:Api::Resource::Transport
encoder: encode_request
decoder: decode_response
Expand Down
6 changes: 3 additions & 3 deletions products/sql/api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ objects:
self_link: |
projects/{{project}}/instances/{{instance}}/sslCerts/
{{sha1_fingerprint}}
virtual: true # we're not enforcing state as it is all server-side driven.
readonly: true # we're not enforcing state as it is all server-side driven.
description: |
Represents an SSL certificate created for a Cloud SQL instance. To use the
SSL certificate you must have the SSL Client Certificate and the
Expand Down Expand Up @@ -451,7 +451,7 @@ objects:
self_link_query: !ruby/object:Api::Resource::ResponseList
kind: 'sql#flagsList'
items: 'items'
virtual: true
readonly: true
properties:
- !ruby/object:Api::Type::Array
name: 'allowedStringValues'
Expand Down Expand Up @@ -506,7 +506,7 @@ objects:
items: 'items'
identity:
- tier
virtual: true
readonly: true
parameters:
- !ruby/object:Api::Type::String
name: 'tier'
Expand Down
12 changes: 6 additions & 6 deletions provider/ansible.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def python_type(prop)
prop = Module.const_get(prop).new('') unless prop.is_a?(Api::Type)
# All ResourceRefs are dicts with properties.
if prop.is_a? Api::Type::ResourceRef
return 'str' if prop.resource_ref.virtual
return 'str' if prop.resource_ref.readonly
return 'dict'
end
PYTHON_TYPE_FROM_MM_TYPE.fetch(prop.class.to_s, 'str')
Expand Down Expand Up @@ -181,7 +181,7 @@ def rrefs_in_link(link, object)
props_in_link = link.scan(/{([a-z_]*)}/).flatten
(object.parameters || []).select do |p|
props_in_link.include?(Google::StringUtils.underscore(p.name)) && \
p.is_a?(Api::Type::ResourceRef) && !p.resource_ref.virtual
p.is_a?(Api::Type::ResourceRef) && !p.resource_ref.readonly
end.any?
end

Expand All @@ -192,7 +192,7 @@ def resourceref_hash_for_links(link, object)
# Select a resourceref if it exists.
rref = (object.parameters || []).select do |prop|
Google::StringUtils.underscore(prop.name) == p && \
prop.is_a?(Api::Type::ResourceRef) && !prop.resource_ref.virtual
prop.is_a?(Api::Type::ResourceRef) && !prop.resource_ref.readonly
end
if rref.any?
[
Expand All @@ -217,10 +217,10 @@ def emit_link_var_args(url, extra_data)
].compact
end

# Returns a list of all first-level ResourceRefs that are not virtual
def nonvirtual_rrefs(object)
# Returns a list of all first-level ResourceRefs that are not readonly
def nonreadonly_rrefs(object)
object.all_resourcerefs
.reject { |prop| prop.resource_ref.virtual }
.reject { |prop| prop.resource_ref.readonly }
end

# Converts a path in the form a/b/c/d into ['a', 'b', 'c', 'd']
Expand Down
6 changes: 3 additions & 3 deletions provider/ansible/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,23 +128,23 @@ def request_output(prop, hash_name, module_name)
"#{hash_name}.get(#{quote_string(prop.out_name)}, [])",
", #{module_name}).to_request()"
].join
elsif prop.is_a?(Api::Type::ResourceRef) && !prop.resource_ref.virtual
elsif prop.is_a?(Api::Type::ResourceRef) && !prop.resource_ref.readonly
prop_name = Google::StringUtils.underscore(prop.name)
[
"replace_resource_dict(#{hash_name}",
".get(#{unicode_string(prop_name)}, {}), ",
"#{quote_string(prop.imports)})"
].join
elsif prop.is_a?(Api::Type::ResourceRef) && \
prop.resource_ref.virtual && prop.imports == 'selfLink'
prop.resource_ref.readonly && prop.imports == 'selfLink'
func = "#{Google::StringUtils.underscore(prop.resource)}_selflink"
[
"#{func}(#{hash_name}.get(#{quote_string(prop.out_name)}),",
"#{module_name}.params)"
].join(' ')
elsif prop.is_a?(Api::Type::Array) && \
prop.item_type.is_a?(Api::Type::ResourceRef) && \
!prop.item_type.resource_ref.virtual
!prop.item_type.resource_ref.readonly
prop_name = Google::StringUtils.underscore(prop.name)
[
"replace_resource_dict(#{hash_name}",
Expand Down
2 changes: 1 addition & 1 deletion provider/ansible/resourceref.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ module Ansible
module ResourceRef
# Builds out a list of statements that handle ResourceRef creation
def resourceref_handlers(object)
rrefs = nonvirtual_rrefs(object)
rrefs = nonreadonly_rrefs(object)
return unless rrefs.any?
comments = [
'# Converts data from:',
Expand Down
18 changes: 9 additions & 9 deletions provider/ansible/selflink.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,25 @@
module Provider
module Ansible
# Responsible for building out all logic for checking + assembling selflinks
# on Ansible virtual properties.
# on Ansible readonly properties.
#
# Some virtual properties (regions, zones) will probably be entered as
# Some readonly properties (regions, zones) will probably be entered as
# names by users (that's most intuitive). If a self-link is required,
# the module should assemble that itself.
module SelfLink
# Returns all rrefs that are virtual and require selflinks.
def virtual_selflink_rrefs(object)
# Returns all rrefs that are readonly and require selflinks.
def readonly_selflink_rrefs(object)
object.all_resourcerefs
.select { |prop| prop.resource_ref.virtual }
.select { |prop| prop.resource_ref.readonly }
.select { |prop| prop.imports == 'selfLink' }
end

# Build out functions that will check + create selflinks.
def selflink_functions(object)
virtuals = virtual_selflink_rrefs(object).map(&:resource_ref)
.uniq
virtuals.map do |virt|
if virt == virtuals.last
readonlys = readonly_selflink_rrefs(object).map(&:resource_ref)
.uniq
readonlys.map do |virt|
if virt == readonlys.last
lines(selflink_function(virt))
else
lines(selflink_function(virt), 1)
Expand Down
4 changes: 2 additions & 2 deletions provider/puppet/test_manifest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ def generate_object(object, title, kind, seed, extra)
credential: "'cred#{seed}'"
}.merge(extra)

# Puppet does not like when virtual resources have an ensure property
extra.delete(:ensure) if object.virtual
# Puppet does not like when readonly resources have an ensure property
extra.delete(:ensure) if object.readonly

[
"#{object.out_name} { '#{title}':",
Expand Down
Loading

0 comments on commit bdc8ab7

Please sign in to comment.