Skip to content
This repository was archived by the owner on Jun 17, 2025. It is now read-only.

Conversation

@arya-girish-k
Copy link

Description

This PR is to update the KMS auth policy so its scope to the exact KMS key.
Git Issue

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content

This PR is to update the KMS auth policy so its scope to the exact KMS key.

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

For mergers

  • Use a conventional commit message to set the release level. Follow the guidelines.
  • Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
  • Use the Squash and merge option.

@arya-girish-k
Copy link
Author

/run pipeline

@arya-girish-k arya-girish-k changed the title 11345 scope kms feat: Scope the KMS auth policy to the exact KMS key Nov 21, 2024
@arya-girish-k
Copy link
Author

The Upgrade test fails due to the re-creation of the auth policy, however since we are using create_before_destroy = true there will be no disruption to key access so skipping upgrade test.

[1558](https://github.com/terraform-ibm-modules/terraform-ibm-icd-edb/actions/runs/11952416333/job/33318288008#step:7:1559)
    tests.go:186: 
[1559](https://github.com/terraform-ibm-modules/terraform-ibm-icd-edb/actions/runs/11952416333/job/33318288008#step:7:1560)
        	Error Trace:	/go/pkg/mod/github.com/terraform-ibm-modules/[email protected]/testhelper/tests.go:186
[1560](https://github.com/terraform-ibm-modules/terraform-ibm-icd-edb/actions/runs/11952416333/job/33318288008#step:7:1561)
        	            				/go/pkg/mod/github.com/terraform-ibm-modules/[email protected]/testhelper/tests.go:766
[1561](https://github.com/terraform-ibm-modules/terraform-ibm-icd-edb/actions/runs/11952416333/job/33318288008#step:7:1562)
        	            				/__w/terraform-ibm-icd-edb/terraform-ibm-icd-edb/tests/pr_test.go:112
[1562](https://github.com/terraform-ibm-modules/terraform-ibm-icd-edb/actions/runs/11952416333/job/33318288008#step:7:1563)
        	Error:      	Should be false
[1563](https://github.com/terraform-ibm-modules/terraform-ibm-icd-edb/actions/runs/11952416333/job/33318288008#step:7:1564)
        	Test:       	TestRunUpgradeCompleteExample
[1564](https://github.com/terraform-ibm-modules/terraform-ibm-icd-edb/actions/runs/11952416333/job/33318288008#step:7:1565)
        	Messages:   	Resource(s) identified to be destroyed 
[1565](https://github.com/terraform-ibm-modules/terraform-ibm-icd-edb/actions/runs/11952416333/job/33318288008#step:7:1566)
        	            	Name: kms_policy
[1566](https://github.com/terraform-ibm-modules/terraform-ibm-icd-edb/actions/runs/11952416333/job/33318288008#step:7:1567)
        	            	Address: module.enterprise_db.ibm_iam_authorization_policy.kms_policy[0]
[1567](https://github.com/terraform-ibm-modules/terraform-ibm-icd-edb/actions/runs/11952416333/job/33318288008#step:7:1568)
        	            	Actions: [create delete]
[1568](https://github.com/terraform-ibm-modules/terraform-ibm-icd-edb/actions/runs/11952416333/job/33318288008#step:7:1569)
        	            	DIFF:
[1569](https://github.com/terraform-ibm-modules/terraform-ibm-icd-edb/actions/runs/11952416333/job/33318288008#step:7:1570)
        	            	  Before: 
[1570](https://github.com/terraform-ibm-modules/terraform-ibm-icd-edb/actions/runs/11952416333/job/33318288008#step:7:1571)
        	            		{"id":"cc5c39a6-8ba7-412e-abe0-0dde7b4a15c8","resource_attributes":"SECURE_VALUE_HIDDEN_HASH:-a62e507cc0d98292a5a103fbfde394bf5d6ed3268ed74d55386a4440","source_resource_instance_id":"","source_resource_type":"","source_service_account":"abac0df06b644a9cabc6e44f55b3880e","subject_attributes":"SECURE_VALUE_HIDDEN_HASH:-5c2f31e6cbe717de6fb4a68d8adba64beda457c54c57bede1840ba48","target_resource_group_id":"","target_resource_instance_id":"7504f55e-4fbc-46ca-857c-ebc0feec681c","target_resource_type":"","target_service_name":"kms","transaction_id":"7ad4bcc2dbc64bab849518695edf2810"}```

@arya-girish-k
Copy link
Author

/run pipeline

Copy link
Member

@imprateeksh imprateeksh left a comment

Choose a reason for hiding this comment

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

Release notes should be concise and descriptive, without PR-specific details. Please modify to focus on changes done.

image

source_service_name = "databases-for-enterprisedb"
source_resource_group_id = var.resource_group_id
roles = ["Reader"]
description = "Allow all Enterprise db instances in the resource group ${var.resource_group_id} to read from the ${local.kms_service} instance GUID ${var.existing_kms_instance_guid}"
Copy link
Member

Choose a reason for hiding this comment

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

To maintain consistency, this can be modified as -

to read the ${local.kms_service} key ${local.kms_key_id} from the instance ${var.existing_kms_instance_guid}"

@arya-girish-k
Copy link
Author

As suggested currently paused on working in this issue until the Elastic search PR is completed, as the same changes needs to be integrated into this one.

@arya-girish-k
Copy link
Author

@ocofaigh ,As per discussion some more issues will be created related to this issue.And also In order to ensure consistency across our ICD modules / DA should make the same changes as per Elastic search module which is handling by @jor2 .
As of now closing this PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants