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

fix: How do I create Organization API Key with Organization Billing Admin permission and Project Read Only for projects #1369

Merged
merged 14 commits into from
Aug 11, 2023

Conversation

andreaangiolillo
Copy link
Collaborator

@andreaangiolillo andreaangiolillo commented Aug 4, 2023

Description

JIRA ticket: INTMDB-898

Link to any related issue(s): #1278

Type of change:

  • Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
  • New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR.
  • Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR.
  • This change requires a documentation update
  • Documentation fix/enhancement

Required Checklist:

  • I have signed the MongoDB CLA
  • I have read the contribution guidelines
  • I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code

Further comments

I locally tested the provider with the following configuration files and checked that the resource was successfully created, the terraform plan did not return any changes and the terraform destroy worked as expected.

resource "mongodbatlas_project" "atlas-project" {
  name   = "Issue-1278"
  org_id = "60ddf55c27a5a20955a707d7"
}

resource "mongodbatlas_project_api_key" "api_1" {
  description = "test api_key multi"
  project_id  = mongodbatlas_project.atlas-project.id

  project_assignment {
    project_id = mongodbatlas_project.atlas-project.id
    role_names = ["ORG_BILLING_ADMIN", "GROUP_READ_ONLY"]
  }

  project_assignment {
    project_id = "63dcfc256af00a5934e60924"
    role_names = ["GROUP_READ_ONLY"]
  }

  project_assignment {
    project_id = "64c23af6f133166c39176cbf"
    role_names = ["GROUP_OWNER"]
  }
}
resource "mongodbatlas_project" "atlas-project" {
  name   = "Issue-1278"
  org_id = "60ddf55c27a5a20955a707d7"
}

resource "mongodbatlas_project_api_key" "api_1" {
  description = "test api_key multi"
  project_id  = mongodbatlas_project.atlas-project.id

  project_assignment {
    project_id = mongodbatlas_project.atlas-project.id
    role_names = ["GROUP_OWNER"]
  }

  project_assignment {
    project_id = "63dcfc256af00a5934e60924"
    role_names = ["GROUP_READ_ONLY"]
  }

  project_assignment {
    project_id = "64c23af6f133166c39176cbf"
    role_names = ["GROUP_OWNER"]
  }
}
Running tool: /Users/andrea.angiolillo/.asdf/shims/go test -timeout 300000s -run ^TestAccConfigRSProjectAPIKey_OrgRoles$ github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas

ok  	github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas	20.060s

@andreaangiolillo andreaangiolillo changed the title INTMDB-898: fixing fix: INTMDB-898: fixing Aug 4, 2023
@andreaangiolillo andreaangiolillo changed the title fix: INTMDB-898: fixing fix: How do I create Organization API Key with Organization Billing Admin permission and Project Read Only for projects Aug 9, 2023
@andreaangiolillo andreaangiolillo marked this pull request as ready for review August 9, 2023 10:49
@andreaangiolillo andreaangiolillo requested a review from a team as a code owner August 9, 2023 10:49
Copy link
Collaborator

@Zuhairahmed Zuhairahmed left a comment

Choose a reason for hiding this comment

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

Few doc comments below, also @andreaangiolillo can you confirm that this update includes no breaking changes?

website/docs/r/project_api_key.html.markdown Show resolved Hide resolved
website/docs/r/project_api_key.html.markdown Outdated Show resolved Hide resolved
@andreaangiolillo
Copy link
Collaborator Author

can you confirm that this update includes no breaking changes?

@Zuhairahmed I tested the changes with the following configuration which contains the examples in the doc and they were all successful. Is there any other scenario that you think I am missing? Thanks

resource "mongodbatlas_project" "atlas-project" {
  name   = "Issue-1278"
  org_id = "60ddf55c27a5a20955a707d7"
}

resource "mongodbatlas_project_api_key" "test1" {
  description = "Description of your API key"
  project_id  = mongodbatlas_project.atlas-project.id
  role_names  = ["GROUP_OWNER"]
}

resource "mongodbatlas_project_api_key" "test22" {
  description = "Description of your API key"
  project_id  = mongodbatlas_project.atlas-project.id

  project_assignment {
    project_id = mongodbatlas_project.atlas-project.id
    role_names = ["GROUP_READ_ONLY", "GROUP_OWNER"]
  }

  project_assignment {
    project_id = "63dcfc256af00a5934e60924"
    role_names = ["GROUP_READ_ONLY"]
  }
}

resource "mongodbatlas_project_api_key" "api_1" {
  description = "test api_key multi"
  project_id  = mongodbatlas_project.atlas-project.id

  project_assignment {
    project_id = mongodbatlas_project.atlas-project.id
    role_names = ["ORG_BILLING_ADMIN", "GROUP_READ_ONLY"]
  }

  project_assignment {
    project_id = "63dcfc256af00a5934e60924"
    role_names = ["GROUP_READ_ONLY"]
  }

  project_assignment {
    project_id = "64c23af6f133166c39176cbf"
    role_names = ["GROUP_OWNER"]
  }
}

Copy link
Collaborator

@maastha maastha left a comment

Choose a reason for hiding this comment

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

LGTM

orgReadOnlyRole = "ORG_READ_ONLY"
)

var orgRoleProvided = false
Copy link
Member

Choose a reason for hiding this comment

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

is there a risk in having this global variable being used in separate operations? Set during create, and then used during read and delete.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, I don't this is an issue but I renamed the var to make it more specific to the project API key resource

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated

Copy link
Member

Choose a reason for hiding this comment

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

My only doubt is that this might work during acceptance tests as the test framework maintains one plugin gRPC server for the duration of each test case, while in normal Terraform operations the plugin server starts and stops (reference).
Approving the PR, I would just try a local build to be sure org roles are supported with no issue.

Copy link
Collaborator Author

@andreaangiolillo andreaangiolillo Aug 10, 2023

Choose a reason for hiding this comment

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

is there a risk in having this global variable being used in separate operations? Set during create, and then used during read and delete.

I think I misread this comment the first time. The reason why I defined this as a global var was to use it in the create and read operations as I need to know if the user has provided any ORG roles in the config:

The main reason for this is that the CREATE endpoint creates an org API key with ORG_READ_ONLY role if the user did not provide any ORG_ROLES as input. In this scenario, the READ will return the ORG_READ_ONLY role (in addition to the GROUP_* roles) even if the user provided only GROUP roles

while in normal Terraform operations the plugin server starts and stops (reference).

In this case, the READ operation is called as last step of the CREATE so we do not hit this issue

Copy link
Collaborator Author

@andreaangiolillo andreaangiolillo Aug 10, 2023

Choose a reason for hiding this comment

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

I tested the changes with the Terraform configurations provided in the description of the PR ( ORG_READ_ONLY is there) and in this comment #1369 (comment). Let me know if I should test locally another scenario

Copy link
Member

Choose a reason for hiding this comment

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

I tested with the following example:

resource "mongodbatlas_project_api_key" "api_1" {
  description = "test api_key multii"
  project_id  = "<project_id>"

  project_assignment {
    project_id = "<project_id>"
    role_names = ["ORG_READ_ONLY", "GROUP_READ_ONLY"]
  }
}
  • terraform apply creates the api key successfully.
  • then running terraform plan outputs incorrect diff showing that "ORG_READ_ONLY" will be added.

I believe the projectAPIKeyOrgRoleProvided is not preserving its value during the create and following read operation done in terraform plan, so it filters out the org role.

Let me know if you are able to reproduce the same, it is indeed an edge case but might be worth addressing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not able to reproduce the issue

apply

tfa                                                                                                  6s  1.20.33.1.116.18.0 bazel 5.4.0 ﴃ cloud-local
mongodbatlas_project.atlas-project: Refreshing state... [id=64d4acf6bf469a0c1549c869]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # mongodbatlas_project_api_key.api_1 will be created
  + resource "mongodbatlas_project_api_key" "api_1" {
      + api_key_id  = (known after apply)
      + description = "test api_key multii"
      + id          = (known after apply)
      + private_key = (sensitive value)
      + project_id  = "64d4acf6bf469a0c1549c869"
      + public_key  = (known after apply)

      + project_assignment {
          + project_id = "64d4acf6bf469a0c1549c869"
          + role_names = [
              + "GROUP_READ_ONLY",
              + "ORG_READ_ONLY",
            ]
        }
    }

Plan: 1 to add, 0 to change, 0 to destroy.
mongodbatlas_project_api_key.api_1: Creating...
mongodbatlas_project_api_key.api_1: Creation complete after 1s [id=YXBpX2tleV9pZA==:NjRkNGFkMWJlODA3YjAzNWZjMjlmNTY2-cHJvamVjdF9pZA==:NjRkNGFjZjZiZjQ2OWEwYzE1NDljODY5]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

plan:

tf plan                                                                                              3s  1.20.33.1.116.18.0 bazel 5.4.0 ﴃ cloud-local
mongodbatlas_project.atlas-project: Refreshing state... [id=64d4acf6bf469a0c1549c869]
mongodbatlas_project_api_key.api_1: Refreshing state... [id=YXBpX2tleV9pZA==:NjRkNGFkMWJlODA3YjAzNWZjMjlmNTY2-cHJvamVjdF9pZA==:NjRkNGFjZjZiZjQ2OWEwYzE1NDljODY5]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was able to reproduce the issue: the reason why it did not "fail" during the plan is that I was using the provider in debug mode. Thanks for the discussion here. I will see if I find a way around

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately, I have not found a way to allow the use of ORG_READ_ONLY since it is automatically added when ORG roles are not provided. As a result, I updated the logic to now allow the ORG_READ_ONLY for project assignments and update the documentation to explain why. This should not be a breaking change since customers were not able to use this resource with org roles.

website/docs/r/project_api_key.html.markdown Show resolved Hide resolved

~> **NOTE:** The organization level roles can be defined only in the first `project_assignment` element.

~> **NOTE:** The `ORG_READ_ONLY` role at the organization level is invalid in this context. When the `project_assignment``` lacks organizational roles, the `mongodbatlas_project_api_key` resource generates an organization API key with the `ORG_READ_ONLY` role and associates it with `GROUP_*` roles. Consequently, the resource does not permit the use of `ORG_READ_ONLY` to ensure consistency between configuration and state.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI

Comment on lines +525 to +537
func (apiKey *APIProjectAssignmentKeyInput) validateOrgKeyRoles() error {
// When the user does not provide org roles
// the API key POST endpoing creates an org api key with
// the role ORG_READ_ONLY. We want to remove this from the state
// to avoid differences between config and state
for _, r := range apiKey.RoleNames {
if r == orgReadOnlyRole {
return fmt.Errorf(`%[1]s is not an allowed role for the resource. Remove %[1]s from the roles and run terraform apply again. Check out the resource documentation to know more`, orgReadOnlyRole)
}
}

return nil
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

validating that the user did not provide ORG_READ_ONLY

Copy link
Member

@AgustinBettati AgustinBettati left a comment

Choose a reason for hiding this comment

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

LGTM

@andreaangiolillo andreaangiolillo merged commit a7c4fdc into master Aug 11, 2023
19 checks passed
@andreaangiolillo andreaangiolillo deleted the INTMDB-898_3 branch August 11, 2023 08:53
andreaangiolillo added a commit that referenced this pull request Aug 25, 2023
…illing Admin permission and Project Read Only for projects (#1369)"

This reverts commit a7c4fdc.
andreaangiolillo added a commit that referenced this pull request Aug 25, 2023
…illing Admin permission and Project Read Only for projects (#1369)" (#1416)

This reverts commit a7c4fdc.
andreaangiolillo added a commit that referenced this pull request Sep 7, 2023
…illing Admin permission and Project Read Only for projects (#1369)" (#1416)

This reverts commit a7c4fdc.
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.

4 participants