refactor: Update prefix logic in DA

[arya-girish-k]

Description

Refactored the code logic for prefix variable by adding default value for prefix and marked as required in catalog manifest.Also allowed prefix to be "" (empty string) for advanced users.
#11959

Release required?

• No release
• Patch release (x.x.X)
• Minor release (x.X.x)
• Major release (X.x.x)

Release notes content

Add the default value for prefix variable and mark as required in catalog manifest. Also allowed prefix to be "" (empty string ). The addition of a default value for the prefix variable has caused a failure in the TestRunUpgradeDASolution. However, if users remove the prefix, it will not result in a breaking change.

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]

/run pipeline

[arya-girish-k]

Please find the Plan output .
https://github.ibm.com/GoldenEye/issues/issues/11959#issuecomment-102616131

[arya-girish-k]

The TestRunUpgradeDASolution is failing due to the addition of a default value for the prefix variable.
https://github.com/terraform-ibm-modules/terraform-ibm-event-notifications/actions/runs/12862137948/job/35856712474#step:7:1164

As per the test terraform will execute the following actions:

Update the resource_group, kms_policy , cos_policy, en_kms_integration, cos_instance, en_cos_integration and en_instance

Destroy the existing kms_key, kms_key_ring, cos_key, cos_key_ring ,root_key_policy resources and cos_bucket.

Create new instances of kms_key, kms_key_ring, cos_key, cos_key_ring and root_key_policy resources.

Since removing the prefix won't result in a breaking change for the user, So as per the discussion it is fine to skip the upgrade test in this scenario.

[arya-girish-k]

/run pipeline

[arya-girish-k]

/run pipeline

[arya-girish-k]

Plan output for reference:

If providing prefix as dev

  �cos_key_name                     = "dev-en-cos-key"
  �cos_key_ring_name                = "dev-en-cos-key-ring"
  �en_key_name                      = "dev-en-key"
  �en_key_ring_name                 = "dev-en-key-ring"
  �event_notification_instance_name = "dev-base-event-notifications"
  �resource_group_name              = "dev-aryrg"

if providing prefix as null or empty string ""

�cos_key_name                     = "en-cos-key"
�cos_key_ring_name                = "en-cos-key-ring"
�en_key_name                      = "en-key"
�en_key_ring_name                 = "en-key-ring"
�event_notification_instance_name = "base-event-notifications"
�resource_group_name              = "aryrg"
``

[shemau]

The justification for skipping the upgrade test appears to be because it will delete resources. That is not a valid justification, the test is specifically to catch that case.

In a simple use case the consumer uses the DA from the module in schematics and deploys and starts using the infrastructure. To keep current they upgrade the version and it deletes the event notifications and cos instance and re-creates new ones.
Further keys are usually used to encrypt data at rest. Deleting a key and creating a new one leaves no way to decrypt the existing data. At the very least this leads to an unpredictable upgrade, the destroy may not work or it may work and the data can no longer be decrypted.

[maheshwarishikha]

The justification for skipping the upgrade test appears to be because it will delete resources. That is not a valid justification, the test is specifically to catch that case.

In a simple use case the consumer uses the DA from the module in schematics and deploys and starts using the infrastructure. To keep current they upgrade the version and it deletes the event notifications and cos instance and re-creates new ones. Further keys are usually used to encrypt data at rest. Deleting a key and creating a new one leaves no way to decrypt the existing data. At the very least this leads to an unpredictable upgrade, the destroy may not work or it may work and the data can no longer be decrypted.

@shemau upgrade tests are failing because we are changing the default value of prefix variable.

• If an user is upgrading its infrastructure, they should use the same prefix as before and then nothing will be destroyed and recreated. They can provide null too if nothing was provided before.
• This PR does not have any other code change as such that will cause destroying any existing infrastructure. So should be fine.

Let me know your thoughts.

[shemau]

This appears to be consistent with what is being done elsewhere, eg. terraform-ibm-modules/terraform-ibm-icd-elasticsearch#388.

There are use cases, where using the solution without specifying a prefix will see the service instance deleted and re-created.

[maheshwarishikha]

@arya-girish-k would be good if we can use try(...) as done in other PRs instead of comparing (var.prefix != null && var.prefix != "") everywhere.

[arya-girish-k]

/run pipeline

[arya-girish-k]

/run pipeline

[terraform-ibm-modules-ops]

🎉 This PR is included in version 1.16.1 🎉

The release is available on:

• GitHub release
• v1.16.1

Your semantic-release bot 📦🚀