feat : scope KMS auth policy to the exact KMS key #530

arya-girish-k · 2024-11-19T15:38:20Z

Description

This PR is to scope the KMS auth policy 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

Scope the KMS auth policy 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 · 2024-11-19T15:39:19Z

/run pipeline

arya-girish-k · 2024-11-27T17:46:01Z

/run pipeline

arya-girish-k · 2024-11-28T05:50:33Z

/run pipeline

arya-girish-k · 2024-11-28T06:21:57Z

/run pipeline

arya-girish-k · 2024-11-28T06:34:16Z

/run pipeline

arya-girish-k · 2024-11-28T10:58:59Z

/run pipeline

arya-girish-k · 2024-11-28T14:38:44Z

/run pipeline

arya-girish-k · 2024-11-29T10:04:13Z

/run pipeline

arya-girish-k · 2024-11-29T11:19:16Z

/run pipeline

arya-girish-k · 2024-11-29T13:13:40Z

/run pipeline

…e-kms

arya-girish-k · 2024-12-02T05:50:08Z

/run pipeline

Ak-sky · 2024-12-02T12:45:41Z

Pipeline failing due to the below error but could not find any existing policy id 51eb33e6-22cf-46b5-9cb1-915a78079b9a in dev acc.

TestRunFSCloudExample 2024-12-02T05:57:50Z retry.go:99: Returning due to fatal error: FatalError{Underlying: error while running command: exit status 1; ╷
Error: [0m Error: [ERROR] Error creating authorization policy: The policy wasn't created because an access policy with identical attributes and roles already exists. Please update the rule in the existing policy (51eb33e6-22cf-46b5-9cb1-915a78079b9a), or update the one you're trying to assign to include a different attribute assignment. {

Ak-sky · 2024-12-02T12:45:47Z

/run pipeline

Ak-sky · 2024-12-02T13:22:47Z

Looks like PR will fail again as same auth policy is getting created, @arya-girish-k can you please check the logic once?

  + keys_check = {
      + key1 = {
          + instance       = "e6dce284-e80f-46e1-a3c1-830f7adff7a9"
          + kms_account_id = "abac0df06b644a9cabc6e44f55b3880e"
          + kms_key_id     = "76170fae-4e0c-48c3-8ebe-326059ebb533"
          + kms_scope      = "a/abac0df06b644a9cabc6e44f55b3880e"
          + kms_service    = "hs-crypto"
          + resource_type  = "key"
        }
      + key2 = {
          + instance       = "e6dce284-e80f-46e1-a3c1-830f7adff7a9"
          + kms_account_id = "abac0df06b644a9cabc6e44f55b3880e"
          + kms_key_id     = "76170fae-4e0c-48c3-8ebe-326059ebb533"
          + kms_scope      = "a/abac0df06b644a9cabc6e44f55b3880e"
          + kms_service    = "hs-crypto"
          + resource_type  = "key"
        }
    }

Ak-sky · 2024-12-02T13:59:26Z

@arya-girish-k, you need to see if backup_encryption_key_crn is same as kms_key_crn, then you do not need to create 2 auth policies but then you also need to check if same kms_key_crn is used as backup_encryption_key_crn then it should be supported in that region, refer below comment.

terraform-ibm-icd-postgresql/main.tf

Line 18 in 7ad7d55

    
           # If no value passed for 'backup_encryption_key_crn' use the value of 'kms_key_crn' and perform validation of 'kms_key_crn' to check if region is supported by backup encryption key.

arya-girish-k · 2024-12-06T14:15:53Z

/run pipeline

arya-girish-k · 2024-12-06T14:57:56Z

Added a parser for KMS and backup encryption keys, along with an authorization policy resource for backup encryption. Also resolved pre-commit errors. Hence retriggering the pipeline.

arya-girish-k · 2024-12-06T14:59:59Z

/run pipeline

arya-girish-k · 2024-12-06T15:12:49Z

Pipeline(CRA scan) is failed because of this error

│ Error: Invalid count argument
│ 
│   on ../../main.tf line 107, in module "backup_kms_crn_parser":
│  107:   count   = local.create_backup_auth_policy
│ 
│ The "count" value depends on resource attributes that cannot be determined
│ until apply, so Terraform cannot predict how many instances will be
│ created. To work around this, use the -target argument to first apply only
│ the resources that the count depends on.
╵
Error: Process completed with exit code 1.```

arya-girish-k · 2024-12-09T11:57:08Z

Updated the validation code in main.tf .Hence retriggering the pipeline

arya-girish-k · 2024-12-09T11:57:18Z

/run pipeline

arya-girish-k · 2024-12-09T12:47:48Z

Resolved the pre-commit error .Re-triggering the pipeline.

arya-girish-k · 2024-12-09T12:48:34Z

/run pipeline

arya-girish-k · 2024-12-09T15:11:01Z

As expected, 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.

    terraform.go:238: 
        	Error Trace:	/go/pkg/mod/github.com/terraform-ibm-modules/[email protected]/testhelper/terraform.go:238
        	            				/go/pkg/mod/github.com/terraform-ibm-modules/[email protected]/testhelper/tests.go:620
        	            				/__w/terraform-ibm-icd-postgresql/terraform-ibm-icd-postgresql/tests/pr_test.go:167
        	Error:      	Should be false
        	Test:       	TestRunStandardUpgradeSolution
        	Messages:   	Resource(s) identified to be destroyed 
        	            	Name: kms_policy
        	            	Address: module.postgresql_db.module.postgresql_db.ibm_iam_authorization_policy.kms_policy[0]
        	            	Actions: [create delete]
        	            	DIFF:
        	            	  Before: 
        	            		{"description":"Allow all ICD Postgres instances in the resource group 2be8e5c7879a43a4afdda20ab9c22852 to read from the hs-crypto instance GUID e6dce284-e80f-46e1-a3c1-830f7adff7a9","id":"72d17db0-bcc4-4971-b305-046b32b050a5","resource_attributes":"SECURE_VALUE_HIDDEN_HASH:-a234ec499736fd2e4a35bbcd3efb473fbc14095c4d280a404e866c82","source_resource_instance_id":"","source_resource_type":"","source_service_account":"abac0df06b644a9cabc6e44f55b3880e","subject_attributes":"SECURE_VALUE_HIDDEN_HASH:-816cc83ba3027c1b6e38e6d10ba26f5ab4fc2440c3d9d4cada474827","target_resource_group_id":"","target_resource_instance_id":"e6dce284-e80f-46e1-a3c1-830f7adff7a9","target_resource_type":"","target_service_name":"hs-crypto","transaction_id":"04212b8fdc95481cb6820bfa026b3050"}
        	            	  After: 
        	            		{"description":"Allow all ICD Postgres instances in the resource group 2be8e5c7879a43a4afdda20ab9c22852 to read from the hs-crypto instance GUID e6dce284-e80f-46e1-a3c1-830f7adff7a9.","resource_attributes":"SECURE_VALUE_HIDDEN_HASH:-7eed2c799fa6472bbda0c258329756e2de64dd8f51e7eccfb6ef1565"}
        	            	
        	            	Change Detail:
        	            	{
        	            	  "actions": [
        	            	    "create",
        	            	    "delete"
        	            	  ],
        	            	  "after": {
        	            	    "description": "Allow all ICD Postgres instances in the resource group 2be8e5c7879a43a4afdda20ab9c22852 to read from the hs-crypto instance GUID e6dce284-e80f-46e1-a3c1-830f7adff7a9.",
        	            	    "resource_attributes": "SECURE_VALUE_HIDDEN_HASH:-b23a09bc211fcf06d1380867f0e2074d43d6169d97768a0fb2713a00",
        	            	    "roles": "SECURE_VALUE_HIDDEN_HASH:-3cb87c221da77a9c9c8ff3d5189bb4a2ccf65a2371e5f3e2e5a2811a",
        	            	    "source_resource_group_id": "2be8e5c7879a43a4afdda20ab9c22852",
        	            	    "source_service_name": "databases-for-postgresql"
        	            	  },
        	            	  "after_sensitive": {
        	            	    "resource_attributes": [
        	            	      {},
        	            	      {},
        	            	      {},
        	            	      {},
        	            	      {}
        	            	    ],
        	            	    "roles": [
        	            	      false
        	            	    ],
        	            	    "subject_attributes": []
        	            	  },
        	            	  "after_unknown": {
        	            	    "id": true,
        	            	    "resource_attributes": [
        	            	      {},
        	            	      {},
        	            	      {},
        	            	      {},
        	            	      {}
        	            	    ],
        	            	    "roles": [
        	            	      false
        	            	    ],
        	            	    "source_resource_instance_id": true,
        	            	    "source_resource_type": true,
        	            	    "source_service_account": true,
        	            	    "subject_attributes": true,
        	            	    "target_resource_group_id": true,
        	            	    "target_resource_instance_id": true,
        	            	    "target_resource_type": true,
        	            	    "target_service_name": true,
        	            	    "transaction_id": true,
        	            	    "version": true
        	            	  },
        	            	  "before": {
        	            	    "description": "Allow all ICD Postgres instances in the resource group 2be8e5c7879a43a4afdda20ab9c22852 to read from the hs-crypto instance GUID e6dce284-e80f-46e1-a3c1-830f7adff7a9",
        	            	    "id": "72d17db0-bcc4-4971-b305-046b32b050a5",
        	            	    "resource_attributes": "SECURE_VALUE_HIDDEN_HASH:-a5c4ee6c0d4a1e17e7f5c45b5297384faa8219c066343c688f5fabbe",
        	            	    "roles": "SECURE_VALUE_HIDDEN_HASH:-2f925fe2e7d7359c571c3d64ba46b29ca34146c7045ffe96f4b72f5e",
        	            	    "source_resource_group_id": "2be8e5c7879a43a4afdda20ab9c22852",
        	            	    "source_resource_instance_id": "",
        	            	    "source_resource_type": "",
        	            	    "source_service_account": "abac0df06b644a9cabc6e44f55b3880e",
        	            	    "source_service_name": "databases-for-postgresql",
        	            	    "subject_attributes": "SECURE_VALUE_HIDDEN_HASH:-1e3043f6df230a786675f4428af787[2358](https://github.com/terraform-ibm-modules/terraform-ibm-icd-postgresql/actions/runs/12235974028/job/34128681519#step:7:2359)1512949ada66f5b35a08f8",
        	            	    "target_resource_group_id": "",
        	            	    "target_resource_instance_id": "e6dce284-e80f-46e1-a3c1-830f7adff7a9",
        	            	    "target_resource_type": "",
        	            	    "target_service_name": "hs-crypto",
        	            	    "transaction_id": "04212b8fdc95481cb6820bfa026b3050",
        	            	    "version": null
        	            	  },
        	            	  "before_sensitive": {
        	            	    "resource_attributes": [
        	            	      {},
        	            	      {},
        	            	      {}
        	            	    ],
        	            	    "roles": [
        	            	      false
        	            	    ],
        	            	    "subject_attributes": []
        	            	  },
        	            	  "replace_paths": [
        	            	    [
        	            	      "resource_attributes"
        	            	    ]
        	            	  ]
        	            	}

arya-girish-k · 2024-12-09T15:33:30Z

@Ak-sky ,TestRunFSCloudExample is failed because of version error.

    tests.go:742: 
        	Error Trace:	/go/pkg/mod/github.com/terraform-ibm-modules/[email protected]/testhelper/tests.go:742
        	            				/go/pkg/mod/github.com/terraform-ibm-modules/[email protected]/testhelper/tests.go:662
        	            				/__w/terraform-ibm-icd-postgresql/terraform-ibm-icd-postgresql/tests/pr_test.go:68
        	Error:      	Expected nil, but got: retry.FatalError{Underlying:(*shell.ErrWithCmdOutput)(0xc000152948)}
        	Test:       	TestRunFSCloudExample
        	Messages:   	Failed%!(EXTRA retry.FatalError=FatalError{Underlying: error while running command: exit status 1; ╷
        	            	│ Error: ---
        	            	│ id: terraform-23dab597
Error:  	            	│ summary: "1 error occurred:\n\t* [ERROR] The version in your configuration file (16)
        	            	│   does not match the version of your remote instance (). Make sure that you have the
        	            	│   same version in your configuration as the version on the remote instance. Learn
        	            	│   more about the versioning policy here: https://cloud.ibm.com/docs/cloud-databases?topic=cloud-databases-versioning-policy
        	            	│   \n\n"
        	            	│ severity: error
        	            	│ resource: ibm_database
        	            	│ operation: CustomizeDiff
        	            	│ component:
        	            	│   name: github.com/IBM-Cloud/terraform-provider-ibm
        	            	│   version: 1.72.0
        	            	│ ---
        	            	│ 
        	            	│ 
        	            	│   with module.postgresql_db.module.postgresql_db.ibm_database.postgresql_db,
        	            	│   on ../../main.tf line 159, in resource "ibm_database" "postgresql_db":
        	            	│  159: resource "ibm_database" "postgresql_db" {
        	            	│ 
        	            	╵})

Tested the issue locally in two ways: first, by commenting out the pg_version in the test for the specific test case, which ran successfully; and second, by setting pg_version ="v16", which resulted in an error.

arya-girish-k · 2024-12-11T03:39:21Z

Provider version is updated .Re-triggering the pipeline.

arya-girish-k · 2024-12-11T03:39:30Z

/run pipeline

maheshwarishikha · 2024-12-11T08:04:25Z

main.tf

-    )
-  ) : null
+  #validation for creating KMS and backup KMS policy
+  create_backup_auth_policy = var.use_default_backup_encryption_key != true && var.backup_encryption_key_crn != null ? 1 : 0


we should say create_backup_kms_auth_policy

maheshwarishikha · 2024-12-11T08:20:59Z

main.tf

+  backup_kms_service    = local.backup_encryption_key_crn != null && length(module.backup_kms_crn_parser) > 0 ? module.backup_kms_crn_parser[0].service_name : null
+  backup_kms_account_id = local.backup_encryption_key_crn != null && length(module.backup_kms_crn_parser) > 0 ? module.backup_kms_crn_parser[0].account_id : null
+  backup_kms_key_id     = local.backup_encryption_key_crn != null && length(module.backup_kms_crn_parser) > 0 ? module.backup_kms_crn_parser[0].resource : null
+  backup_instance       = local.backup_encryption_key_crn != null && length(module.backup_kms_crn_parser) > 0 ? module.backup_kms_crn_parser[0].service_instance : null


We should use var.backup_encryption_key_crn in conditions instead of local.backup_encryption_key_crn
Then we may not require length(module.backup_kms_crn_parser) > 0

arya-girish-k · 2024-12-12T09:52:18Z

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.

ocofaigh · 2024-12-12T14:06:04Z

@arya-girish-k The Elasticsearch PR has been merged, however it contains alot of refactoring work. In order to ensure consistency across our ICD modules / DA, we should make the same changes to Postgres. So I am re-assigning this issue to @jor2 who is already working on the refactoring work as part of #541

feat : scope KMS-policy to the exact KMS key

0c8a8fa

arya-girish-k requested review from rajatagarwal-ibm and Ak-sky as code owners November 19, 2024 15:38

Arya Girish K added 2 commits November 27, 2024 23:09

Added Auth policy to backup encryption key

2b1d174

Update Branch

f7d4bd5

added changes main.tf

dc79d66

latest changes

415495f

Changes in description

36b6011

added variable in cra-config.yaml

a0cc673

added changes related to backup_encryption_key variable

3536343

fixed the cr-config.yaml issue

0c467b8

arya-girish-k and others added 4 commits December 2, 2024 10:19

Merge branch 'main' into 11342-scope-kms

39b60e3

latest changes

0610552

Added Changes in Readme and cra-config.yaml

29d4104

Merge remote-tracking branch 'origin/11342-scope-kms' into 11342-scop…

953ee2f

…e-kms

Arya Girish K added 2 commits December 4, 2024 13:57

Testing

509b863

Added parser and Auth policy resource for Backup encryption key

df12a6d

Resolved Pre-commit errors

5fe8a92

Merge branch 'main' into 11342-scope-kms

79e09e3

Ak-sky mentioned this pull request Dec 9, 2024

fix: change count condition terraform-ibm-modules/terraform-ibm-icd-elasticsearch#360

Closed

6 tasks

Updated validation code in main.tf

ca26e4d

Resolved precommit error

6cf4cb0

SKIP UPGRADE TEST

c9702b2

maheshwarishikha reviewed Dec 11, 2024

View reviewed changes

ocofaigh closed this Dec 12, 2024

ocofaigh deleted the 11342-scope-kms branch December 12, 2024 14:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat : scope KMS auth policy to the exact KMS key #530

feat : scope KMS auth policy to the exact KMS key #530

arya-girish-k commented Nov 19, 2024 •

edited by maheshwarishikha

Loading

arya-girish-k commented Nov 19, 2024

arya-girish-k commented Nov 27, 2024

arya-girish-k commented Nov 28, 2024

arya-girish-k commented Nov 28, 2024

arya-girish-k commented Nov 28, 2024

arya-girish-k commented Nov 28, 2024

arya-girish-k commented Nov 28, 2024

arya-girish-k commented Nov 29, 2024

arya-girish-k commented Nov 29, 2024

arya-girish-k commented Nov 29, 2024

arya-girish-k commented Dec 2, 2024

Ak-sky commented Dec 2, 2024

Ak-sky commented Dec 2, 2024

Ak-sky commented Dec 2, 2024

Ak-sky commented Dec 2, 2024

arya-girish-k commented Dec 6, 2024

arya-girish-k commented Dec 6, 2024

arya-girish-k commented Dec 6, 2024

arya-girish-k commented Dec 6, 2024

arya-girish-k commented Dec 9, 2024

arya-girish-k commented Dec 9, 2024

arya-girish-k commented Dec 9, 2024

arya-girish-k commented Dec 9, 2024

arya-girish-k commented Dec 9, 2024

arya-girish-k commented Dec 9, 2024

arya-girish-k commented Dec 11, 2024

arya-girish-k commented Dec 11, 2024

maheshwarishikha Dec 11, 2024

maheshwarishikha Dec 11, 2024

arya-girish-k commented Dec 12, 2024

ocofaigh commented Dec 12, 2024

feat : scope KMS auth policy to the exact KMS key #530

feat : scope KMS auth policy to the exact KMS key #530

Conversation

arya-girish-k commented Nov 19, 2024 • edited by maheshwarishikha Loading

Description

Release required?

Release notes content

Run the pipeline

Checklist for reviewers

For mergers

arya-girish-k commented Nov 19, 2024

arya-girish-k commented Nov 27, 2024

arya-girish-k commented Nov 28, 2024

arya-girish-k commented Nov 28, 2024

arya-girish-k commented Nov 28, 2024

arya-girish-k commented Nov 28, 2024

arya-girish-k commented Nov 28, 2024

arya-girish-k commented Nov 29, 2024

arya-girish-k commented Nov 29, 2024

arya-girish-k commented Nov 29, 2024

arya-girish-k commented Dec 2, 2024

Ak-sky commented Dec 2, 2024

Ak-sky commented Dec 2, 2024

Ak-sky commented Dec 2, 2024

Ak-sky commented Dec 2, 2024

arya-girish-k commented Dec 6, 2024

arya-girish-k commented Dec 6, 2024

arya-girish-k commented Dec 6, 2024

arya-girish-k commented Dec 6, 2024

arya-girish-k commented Dec 9, 2024

arya-girish-k commented Dec 9, 2024

arya-girish-k commented Dec 9, 2024

arya-girish-k commented Dec 9, 2024

arya-girish-k commented Dec 9, 2024

arya-girish-k commented Dec 9, 2024

arya-girish-k commented Dec 11, 2024

arya-girish-k commented Dec 11, 2024

maheshwarishikha Dec 11, 2024

Choose a reason for hiding this comment

maheshwarishikha Dec 11, 2024

Choose a reason for hiding this comment

arya-girish-k commented Dec 12, 2024

ocofaigh commented Dec 12, 2024

arya-girish-k commented Nov 19, 2024 •

edited by maheshwarishikha

Loading