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

Generate subnetwork for terraform. #276

Merged
merged 19 commits into from
Jun 19, 2018
Merged

Conversation

nat-henderson
Copy link
Contributor

Six notes:

  • Adds a few fields to Subnetwork which were present in Terraform but
    not elsewhere (because they're in beta).
  • Changes terraform so that it's possible to rename a field with child
    fields. Previously, the ordering of overrides was undefined, so it was
    possible to rename a field from 'foo' to 'bar' - and then when it came
    time to apply an override to 'foo.baz', it would crash since there's no
    field called 'foo'. Now we apply overrides in reverse order of level of
    nestedness, so child fields are overriden first.
  • Forces 'id_format' to be a valid import format.
  • Sets subnetwork to be non-updateable, so that ansible/puppet/chef will
    destroy/recreate (and so terraform will use custom-update
    appropriately).
  • Adds beta generation to generate-terraform.sh. By the
    zero-one-infinity rule, the right thing to do is to add a 'beta
    detector' and start automatically picking the version in that script...
    maybe next quarter. :)
  • Removed the ability to update enable_flow_logs. According to the
    docs, this never worked - patch can only update secondaryIpRange,
    role (which does not exist - I filed a bug) and
    allow_subnet_cidr_routes_overlap, which is a field that cannot be set at
    create-time, only updated, and which I decided to omit. In any event,
    no enable_flow_logs updates.
    https://cloud.google.com/compute/docs/reference/rest/beta/subnetworks/patch

[all]

Updates to subnetwork docs, removes broken 'update' function.

[terraform]

Autogenerate Subnetwork.

[puppet]

[puppet-compute]

[puppet-sql]

[puppet-storage]

[chef]

[chef-compute]

[chef-sql]

[chef-storage]

[ansible]

Six notes:
- Adds a few fields to Subnetwork which were present in Terraform but
not elsewhere (because they're in beta).
- Changes terraform so that it's possible to rename a field with child
fields.  Previously, the ordering of overrides was undefined, so it was
possible to rename a field from 'foo' to 'bar' - and then when it came
time to apply an override to 'foo.baz', it would crash since there's no
field called 'foo'.  Now we apply overrides in reverse order of level of
nestedness, so child fields are overriden first.
- Forces 'id_format' to be a valid import format.
- Sets subnetwork to be non-updateable, so that ansible/puppet/chef will
destroy/recreate (and so terraform will use custom-update
appropriately).
- Adds beta generation to generate-terraform.sh.  By the
zero-one-infinity rule, the *right* thing to do is to add a 'beta
detector' and start automatically picking the version in that script...
maybe next quarter.  :)
- Removed the ability to update enable_flow_logs.  According to the
docs, this never worked - patch can only update secondaryIpRange,
role (which does not exist - I filed a bug) and
allow_subnet_cidr_routes_overlap, which is a field that cannot be set at
create-time, only updated, and which I decided to omit.  In any event,
no enable_flow_logs updates.
https://cloud.google.com/compute/docs/reference/rest/beta/subnetworks/patch
@modular-magician
Copy link
Collaborator

I am a robot that works on MagicModules PRs!

I built this PR into one or more PRs on other repositories, and when those are closed, this PR will also be merged and closed.
depends: hashicorp/terraform-provider-google#1661
depends: modular-magician/ansible#30
depends: GoogleCloudPlatform/puppet-google-compute#47
depends: GoogleCloudPlatform/chef-google-compute#17

@modular-magician
Copy link
Collaborator

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit 4cad672) have been included in your existing downstream PRs.

@modular-magician
Copy link
Collaborator

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit d76a4de) have been included in your existing downstream PRs.

@modular-magician
Copy link
Collaborator

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit d775d8b) have been included in your existing downstream PRs.

@modular-magician
Copy link
Collaborator

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit 3dd2a22) have been included in your existing downstream PRs.

@modular-magician
Copy link
Collaborator

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit 74f999c) have been included in your existing downstream PRs.

<% settable_properties.each do |prop| -%>
<%= prop.api_name -%>Prop, err := expand<%= resource_name -%><%= titlelize_property(prop) -%>(d.Get("<%= Google::StringUtils.underscore(prop.name) -%>"), d, config)
if err != nil {
return err
} else if v, ok := d.GetOkExists("<%= Google::StringUtils.underscore(prop.name) -%>"); ok || !reflect.DeepEqual(v, <%= prop.api_name -%>Prop) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let me make sure I understand the second half of the statement. This seems to handle the case where the value was not set (therefore its value will be the zero value) and its value (the zero value) is not equal to what expand produced. So this is for if it wasn't set, but we still do stuff in expand for it? So things like region would be the example case, right? Is there another type of field that would do it? Either way, mind adding a comment (in ruby blocks is fine) explaining this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You got it, and you're 100% right. I will add a comment as soon as I am convinced, myself, that it works. :)

@@ -29,6 +29,7 @@ bundle exec compiler -p products/redis -e terraform -o "${GOPATH}/src/github.com

# Resources that were already using beta APIs before they started being autogenerated
bundle exec compiler -v beta -p products/compute -t Address -e terraform -o "${GOPATH}/src/github.com/terraform-providers/terraform-provider-google/"
bundle exec compiler -v beta -p products/compute -t Subnetwork -e terraform -o "${GOPATH}/src/github.com/terraform-providers/terraform-provider-google/"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: -t takes a list, so you should be able to just do -t Address,Subnetwork (probably should verify this for sure though)

In preparation for testing Ansible linters on CI
@nat-henderson
Copy link
Contributor Author

Bummer, this introduces an issue with SslProxy that looks related - I'm on it.

@modular-magician
Copy link
Collaborator

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit 21372bc) have been included in your existing downstream PRs.

@nat-henderson
Copy link
Contributor Author

Okay, to fix that I did have to introduce the concept analogous to SendEmptyValue, which I don't love, but which I really do think is the only way. Here it is. Tests pass with this code.

@modular-magician
Copy link
Collaborator

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit fa91844) have been included in your existing downstream PRs.

rambleraptor and others added 3 commits June 19, 2018 13:44
Tracked submodules are build/puppet/compute build/puppet/sql build/puppet/storage build/chef/compute build/chef/sql build/chef/storage build/terraform build/ansible.
@modular-magician modular-magician merged commit d27c675 into master Jun 19, 2018
@modular-magician
Copy link
Collaborator

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit cd384c4) have been included in your existing downstream PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants