Skip to content
This repository has been archived by the owner on Sep 16, 2024. It is now read-only.

Feature: Add basic Google Artifact Registry support #39

Merged
merged 4 commits into from
Feb 3, 2022
Merged

Feature: Add basic Google Artifact Registry support #39

merged 4 commits into from
Feb 3, 2022

Conversation

patricklubach
Copy link
Contributor

@patricklubach patricklubach commented Feb 2, 2022

Pull Request

Description

This PR adds basic support for Artifact Registry as on the one hand I need the support and on the other GCR Cleaner supports Artifact Registry too.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

I created a new Google Cloud project, switched to examples/minimal, created a terraform.tfvars file, add all necessary values for required variables. Run terraform init, terraform plan -out=terraform.plan and terraform apply terraform.plan.
When deployment finishes I verify GCR Cleaner Cloud Run and Cloud Scheduler Trigger are there. Trigger Cloud Run triggers and check status of last run was successful.

Checklist:

  • My PR title follows semantics prefix
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings
  • I have run pre-commit hooks pre-commit run -a
  • CI tests are passing

@patricklubach patricklubach changed the title feature: add basic Google Artifact Registry support Feature: add basic Google Artifact Registry support Feb 2, 2022
@patricklubach patricklubach changed the title Feature: add basic Google Artifact Registry support Feature: Add basic Google Artifact Registry support Feb 2, 2022
@anouarchattouna anouarchattouna added enhancement New feature or request good first issue Good for newcomers labels Feb 2, 2022
Copy link
Contributor

@anouarchattouna anouarchattouna left a comment

Choose a reason for hiding this comment

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

Could you please add the google-beta provider configuration to each example's versions.tf file ?

    google-beta = {
      source  = "hashicorp/google-beta"
      version = ">= 4.1.0"
    }

iam.tf Outdated Show resolved Hide resolved
variables.tf Show resolved Hide resolved
variables.tf Show resolved Hide resolved
@patricklubach
Copy link
Contributor Author

Could you please add the google-beta provider configuration to each example's versions.tf file ?

    google-beta = {
      source  = "hashicorp/google-beta"
      version = ">= 4.1.0"
    }

I added them. Please review :)

Copy link
Contributor

@anouarchattouna anouarchattouna left a comment

Choose a reason for hiding this comment

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

LGTM!
Could you just double-check the pre-commit run -a command

@patricklubach
Copy link
Contributor Author

patricklubach commented Feb 2, 2022

@anouarchattouna I double checked the pre-commit run -a command and all checks passed :)
@PascalBourdier @mirakl-admin @nhuray can someone of you review too?

locals.tf Outdated Show resolved Hide resolved
@anouarchattouna anouarchattouna merged commit 8d6ba49 into mirakl:main Feb 3, 2022
mirakl-admin pushed a commit that referenced this pull request Feb 3, 2022
## 1.0.0 (2022-02-03)

### Features

* Add basic Google Artifact Registry support ([#39](#39)) ([8d6ba49](8d6ba49))
* Add support for GCR buckets with uniform_bucket_level_access = true ([#32](#32)) ([16d6232](16d6232))
* First Implementation of GCR Cleaner ([#1](#1)) ([5ad9957](5ad9957))
* Implement dry_run to easily list images to delete ([#34](#34)) ([b5bb78a](b5bb78a))
* Implementing all payload parameters ([#24](#24)) ([dde3658](dde3658))
* Implementing get all repositories of a given project ([#3](#3)) ([f043971](f043971))
* Introduce new payload parameters ([#29](#29)) ([0ea8b25](0ea8b25))

### Bug Fixes

* Adding repos parameter to payload ([#27](#27)) ([0b62c57](0b62c57)), closes [#24](#24)

### Reverts

* Revert "add support for gcr buckets with uniform_bucket_level_access = true (#30)" (#31) ([9eb0fde](9eb0fde)), closes [#30](#30) [#31](#31)
@patricklubach patricklubach deleted the artifact_registry branch February 9, 2022 14:10
@christidis
Copy link
Contributor

christidis commented Jun 29, 2022

@patricklubach / @anouarchattouna / @PascalBourdier

how is this supposed to work for a docker repository in Artifact Registry and set different rules per docker image?

In gar_repositories there is an option for project_id for region and repository name but there is no option for a docker image name. (like there is for example in gcr_repositories).

  gar_repositories = [
    {
      project_id = "foobar-123"
      region     = "europe-west1"
      name       = "myrepo"
      parameters = {
        grace      = "180h"
        keep       = 3
        tag_filter = "^alpha.+$"
      }
    }
  ]

Assumming there are 2 docker image names unser myrepo GAR eg nginx and python how am I supposed to apply different rulesets for each one? Am I losing something?

Also I think this clean_all reference here is incorrect https://github.com/mirakl/terraform-google-gcr-cleaner/blob/main/variables.tf#L140

Update: I have opened this issue #52

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants