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

Added Artifact Registry product #3360

Merged
merged 5 commits into from
Apr 22, 2020

Conversation

ardagnir
Copy link
Contributor

@ardagnir ardagnir commented Apr 9, 2020

Adds support for Artifact Registry

Fixes hashicorp/terraform-provider-google#5928.

Release Note Template for Downstream PRs (will be copied)

`google_artifact_registry_repository` beta-only

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform Beta: Diff ( 10 files changed, 1439 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 231 insertions(+))

@rileykarson rileykarson requested a review from chrisst April 13, 2020 22:09
Copy link
Contributor

@chrisst chrisst left a comment

Choose a reason for hiding this comment

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

Thanks for adding this! I have just a couple minor fixes.

required: true
input: true
- !ruby/object:Api::Type::String
name: 'region'
Copy link
Contributor

Choose a reason for hiding this comment

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

We are moving to using location for resources especially when the url path uses the same naming. It also looks like this may support locations like us or europe based on the output of https://artifactregistry.googleapis.com/v1beta1/projects/{{project}}/locations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing region->location breaks the autogenerated iam code. I tried changing around the settings but it always gives the following error:

Error: Import id "" doesn't match any of the accepted formats: [projects/(?P[^/]+)/locations/(?P[^/]+)/repositories/(?P[^/]+) (?P[^/]+)/(?P[^/]+)/(?P[^/]+) (?P[^/]+)/(?P[^/]+)]

Copy link
Contributor

Choose a reason for hiding this comment

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

You can update the IAM import format to use location as well. See https://github.com/GoogleCloudPlatform/magic-modules/blob/master/products/cloudrun/api.yaml#L241-L245 as an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks! Was going in a completely wrong direction because of the empty string in the error message. Fixed that too.

products/artifactregistry/api.yaml Show resolved Hide resolved
products/artifactregistry/api.yaml Outdated Show resolved Hide resolved
products/artifactregistry/api.yaml Show resolved Hide resolved
products/artifactregistry/terraform.yaml Outdated Show resolved Hide resolved
products/artifactregistry/terraform.yaml Show resolved Hide resolved
@ardagnir ardagnir closed this Apr 15, 2020
@ardagnir ardagnir reopened this Apr 15, 2020
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform Beta: Diff ( 10 files changed, 1439 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 231 insertions(+))

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform Beta: Diff ( 10 files changed, 1433 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 231 insertions(+))

versions:
- !ruby/object:Api::Product::Version
name: beta
base_url: https://artifactregistry.googleapis.com/v1beta1/

Choose a reason for hiding this comment

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

What happens when we upgrade to v1? Do we have multiple version of this, or just change this string?

Copy link
Contributor

Choose a reason for hiding this comment

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

If there are multiple APIs we try to keep multiple versions specified here. That will generate terraform-provider-google-beta which will call into the beta API and terraform-provider-google which will call the v1 endpoints. In the future if fields are added to the beta API that haven't made it to v1 we can support those.

Also fixed incorrect import error message
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))
Terraform Beta: Diff ( 11 files changed, 1441 insertions(+), 4 deletions(-))
TF OiCS: Diff ( 8 files changed, 234 insertions(+))

@ardagnir ardagnir requested a review from chrisst April 22, 2020 22:47
Copy link
Contributor

@chrisst chrisst left a comment

Choose a reason for hiding this comment

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

I added the links to website side-bar and if the build passes it looks good. Thanks!

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 1 insertion(+), 2 deletions(-))
Terraform Beta: Diff ( 12 files changed, 1458 insertions(+), 5 deletions(-))
TF OiCS: Diff ( 8 files changed, 234 insertions(+))

@chrisst chrisst merged commit 2cd17b6 into GoogleCloudPlatform:master Apr 22, 2020
nathkn pushed a commit to nathkn/magic-modules that referenced this pull request May 18, 2020
* Added artifact registry product

* Artifact Registry fixes

* Switched property from region -> location

Also fixed incorrect import error message

* Add artifact registry links to website

Co-authored-by: Chris Stephens <chrisst@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Google Cloud Artifact Registry
5 participants