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

feat: implement the new iam interface in artifact-registry #2606

Conversation

NitriKx
Copy link
Contributor

@NitriKx NitriKx commented Oct 4, 2024

This PR implements the new IAM interface in the atifact-registry module (#2605)

In order to have a consistent interface across the different modules, this update will cause a delete/create of the existing permissions set in the iam variable:

  # module.artifact_registry.google_artifact_registry_repository_iam_binding.authoritative["roles/artifactregistry.reader"] will be created
  + resource "google_artifact_registry_repository_iam_binding" "authoritative" {
      + etag       = (known after apply)
      + id         = (known after apply)
      + location   = "europe"
      + members    = [
          + "allUsers",
        ]
      + project    = "xxredactedxx"
      + repository = "public"
      + role       = "roles/artifactregistry.reader"
    }

  # module.artifact_registry.google_artifact_registry_repository_iam_binding.bindings["roles/artifactregistry.reader"] will be destroyed
  # (because key ["roles/artifactregistry.reader"] is not in for_each map)
  - resource "google_artifact_registry_repository_iam_binding" "bindings" {
      - etag       = "xxredactedxx" -> null
      - id         = "xxredactedxx" -> null
      - location   = "europe" -> null
      - members    = [
          - "allUsers",
        ] -> null
      - project    = "xxredactedxx" -> null
      - repository = "projects/xxredactedxx/locations/europe/repositories/public" -> null
      - role       = "roles/artifactregistry.reader" -> null
    }


Checklist

I applicable, I acknowledge that I have:

  • Read the contributing guide
  • Ran terraform fmt on all modified files
  • Regenerated the relevant README.md files using tools/tfdoc.py
  • Made sure all relevant tests pass

@NitriKx NitriKx force-pushed the bsa-new-iam-interface-artifact-registry branch 2 times, most recently from 2ad1258 to 3171c69 Compare October 4, 2024 08:12
@ludoo
Copy link
Collaborator

ludoo commented Oct 4, 2024

Thanks a lot for this. Can you also add relevant examples and inventories to the README? The contributing guide explains how those work, or ping here if you need a quick leg up.

Copy link
Collaborator

@juliocc juliocc left a comment

Choose a reason for hiding this comment

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

A few comments:

  1. Move the IAM-related variables to variables-iam.tf
  2. Move the IAM-related resources to iam.tf
  3. Add the required dependencies to outputs
  4. Make sure to implement the full interface explained here

@NitriKx NitriKx force-pushed the bsa-new-iam-interface-artifact-registry branch from 3171c69 to 87906a6 Compare October 4, 2024 08:46
@NitriKx NitriKx marked this pull request as ready for review October 4, 2024 08:47
@NitriKx NitriKx requested a review from juliocc October 4, 2024 08:47
@NitriKx NitriKx marked this pull request as draft October 4, 2024 08:48
@NitriKx
Copy link
Contributor Author

NitriKx commented Oct 4, 2024

Working on the tests sorry

Copy link
Collaborator

@juliocc juliocc 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 the quick response :)

Just one small detail with the moved block and we're good to go

@NitriKx NitriKx force-pushed the bsa-new-iam-interface-artifact-registry branch 4 times, most recently from 19a8c19 to 7725959 Compare October 4, 2024 12:43
@NitriKx NitriKx marked this pull request as ready for review October 4, 2024 12:55
@NitriKx NitriKx requested a review from ludoo October 4, 2024 12:56
@NitriKx NitriKx force-pushed the bsa-new-iam-interface-artifact-registry branch from 7725959 to f631f37 Compare October 4, 2024 12:58
@juliocc juliocc enabled auto-merge (squash) October 4, 2024 13:39
@juliocc juliocc merged commit cb234fd into GoogleCloudPlatform:master Oct 4, 2024
14 checks passed
@NitriKx NitriKx deleted the bsa-new-iam-interface-artifact-registry branch October 4, 2024 13:55
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.

3 participants