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

Add convenience field support to Terraform, generate Network #1489

Merged

Conversation

rileykarson
Copy link
Member

Sorry for the # of changes! A few things had to happen for this PR.

Contained, roughly in order is;

  • Add overrides for Terraform
  • Add support for flattening objects
  • Fix broken TF tests, test the actual complex cases (updating, legacy networks)
  • Fix update for Network in api.yaml
  • Clarify docs, remove the old resource-level message because it was outdated

Going from my definition of convenience fields;

convenience fields: When we take a nested object field and top-level it.
useful for nested objects with non-zero defaults.
example: BackendService connection_draining.draining_timeout_sec -> connection_draining_timeout_sec

This is adding support for the most useful subset, when a nested objects has been replaced entirely. Most? examples of convenience fields are a single field inside of an object, and this solution will unblock those cases. Support for nested convenience fields is a TODO, and something we should do if it's necessary, not just for fun.

I looked at using the overrides system to do this, and specifying convenience_name or something similar on the convenience field itself, but that approach didn't work well in the end. Our model for resource schemas relies pretty heavily on 1:1 mappings API <-> tool, and there are a lot of yaks to shave before that could be changed.


[all]

[terraform]

Generate google_compute_network using Magic Modules

[terraform-beta]

[ansible]

[inspec]

@nat-henderson
Copy link
Contributor

This one is a challenge to review without the downstreams - I'll see if I can make sense of them when they come out.

@rileykarson rileykarson force-pushed the convenience-network branch from 4a224b9 to 941b3f0 Compare March 8, 2019 00:20
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
This PR seems not to have generated downstream PRs before, as of 941b3f0.

Pull request statuses

No diff detected in Inspec.

New Pull Requests

I 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.
depends: hashicorp/terraform-provider-google-beta#509
depends: GoogleCloudPlatform/terraform-google-conversion#9
depends: hashicorp/terraform-provider-google#3203
depends: modular-magician/ansible#204

@nat-henderson
Copy link
Contributor

If the network tests pass (and once you fix the quotes that travis is complaining about, and two minor tweaks on the beta / ga modules), I think I trust this to be right.

# Add a deprecation message for a field that's been deprecated in the API
# use the YAML chomping folding indicator (>-) if this is a multiline
# string, as providers expect a single-line one w/o a newline.
attr_reader :deprecation_message
Copy link
Member Author

Choose a reason for hiding this comment

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

@rileykarson rileykarson force-pushed the convenience-network branch from 56ed376 to 0bd5d24 Compare March 8, 2019 01:29
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 0bd5d24.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
Ansible already has an open PR.
No diff detected in Inspec.

New Pull Requests

I didn't open any new pull requests because of this PR.

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 86ec298.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
Ansible already has an open PR.
No diff detected in Inspec.

New Pull Requests

I 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.
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