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

Pack of new modules for Oracle Cloud #50

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

grzesjam
Copy link
Collaborator

@grzesjam grzesjam commented Sep 9, 2021

Container-registry: creates CR and generates permissions
oci-config: parses oci cli config to be used in terraform
s3-compatible: module as drop in replacement of AWS's S3
user: creates user and manages policy in one place

@grzesjam grzesjam added enhancement New feature or request minor Minor changes in PR (according to semver) labels Sep 9, 2021
Container-registry: creates CR and generates permissions
oci-config: parses oci cli config to be used in terraform
s3-compatible: module as drop in replacement of AWS's S3
user: creates user and manages policy in one place
Comment on lines +11 to +23
module "container_registry_api_php" {
source = "./.."

compartment_id = local.tenancy
project_name = local.project_name
environment = local.environment
region = "me-jeddah-1"
suffix = "api"

group_name_for_permission = "${local.project_name}-${local.environment}-CICD"
container_repository_is_immutable = false
container_repository_is_public = false
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a sample resource that consumes the policy output to the example?

Comment on lines 13 to 20
output "policy" {
description = "policies that allow push and pull to this container registry by user"
value = [
"Allow group ${var.group_name_for_permission} to read repos in compartment id ${var.compartment_id} where any { target.repo.name=/${oci_artifacts_container_repository.terraform_container_registry.display_name}*/}",
"Allow group ${var.group_name_for_permission} to use repos in compartment id ${var.compartment_id} where any { target.repo.name=/${oci_artifacts_container_repository.terraform_container_registry.display_name}*/}",
"Allow group ${var.group_name_for_permission} to manage repos in compartment id ${var.compartment_id} where any { target.repo.name=/${oci_artifacts_container_repository.terraform_container_registry.display_name}*/}",
]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. That may be just me, but I'm not a fan of a lot of interpolation in outputs.tf and I'd keep it in main.tf in locals. You also may try to shorten it a bit by using extra locals.
  2. Isn't it too narrow? I.e. what if I want to allow developers to manage the repo, but allow some application role to just pull fresh images?

Something like this:

#variables.tf

variable "policies" {
  type = map(list(string))
}

# main.tf
locals {
  repo_name = oci_artifacts_container_repository.terraform_container_registry.display_name
  repo_url = "${var.region}.ocir.io/${data.oci_objectstorage_namespace.namespace.namespace}/${local.repo_name}"

  policy_actions = ["read", "use", "manage"]
  
  policies = {
    for name, permissions in var.policies : name => flatten([
      for permission in permissions : [
        "Allow ${name} to ${permission} repos in compartment id ${var.compartment_id} where any { target.repo.name=/${local.repo_name}*/}"
      ]
    ])
  }
}

# outputs.tf

output "url" {
  value = local.repo_url
}

output "policies" {
  value = local.policies
}

output "policy" {
  # You can use flatten() to output everything in a single array as well
}

The above should accept something like:

module "container_registry_api_php" {
  policies = {
    "group foo" = ["read", "use", "manage"],
    "group bar" = ["read"]
  }
}

locals {
  resulting_policies = container_registry_api_php.policies
}

# resulting policies are as below:
{
  "group foo" = [
    "Allow group foo to read repos in compartment id some_id where any { target.repo.name=/repo_name*/}",
    "Allow group foo to use repos in compartment id some_id where any { target.repo.name=/repo_name*/}",
    "Allow group foo to manage repos in compartment id some_id where any { target.repo.name=/repo_name*/}",
  ],
  "group bar" = [
    "Allow group bar to read repos in compartment id some_id where any { target.repo.name=/repo_name*/}",
  ],
}

Copy link
Member

Choose a reason for hiding this comment

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

@grzesjam maybe stick (repo) convention output.tf ->outputs.tf and variable.tf ->variables.tf?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@krzysztof-miemiec

  1. I prefer to have output in output, it's much easier to fix i something outputs incorrectly, I don't need to jump between all files. But I'm not sure which approach is more correct
  2. I do understand you with your approach but with it, we expect someone already understanding how permissions in Oracle Cloud work, and what is needed which I don't like, it should "just work".
    How about just ask for "read" and "manage" group name and generate output with "read_permissions" and "full_permissions"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request minor Minor changes in PR (according to semver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants