-
Notifications
You must be signed in to change notification settings - Fork 1
Create Fully Configurable and Security Enforced DA #399
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
Conversation
|
I am unsure if the values in the .catalog-onboard-pipeline.yaml are correct. |
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
|
Only test that is failing is the upgrade test, which is expected. |
|
/run pipeline |
akocbek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure that all changes that are for fully-configurable DA must be applied to security-enforced as well.
cra-config.yaml
Outdated
| TF_VAR_resource_group_name: "geretain-test-redis" | ||
| TF_VAR_existing_resource_group_name: "geretain-test-rabbitmq" | ||
| TF_VAR_provider_visibility: "public" | ||
| TF_VAR_use_ibm_owned_encryption_key: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does fullly-configurable DA have use_ibm_owned_encryption_key input?
sync with Redis, pay attention to the order as well
| "solution", | ||
| "rabbitmq standard", | ||
| "cache", | ||
| "in memory" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove rabbitmq standard" keywords
| "offering_icon_url": "https://raw.githubusercontent.com/terraform-ibm-modules/terraform-ibm-icd-rabbitmq/main/images/rabbitmq_icon.svg", | ||
| "provider_name": "IBM", | ||
| "support_details": "This product is in the community registry, as such support is handled through the originated repo. If you experience issues please open an issue in the repository [https://github.com/terraform-ibm-modules/terraform-ibm-icd-rabbitmq/issues](https://github.com/terraform-ibm-modules/terraform-ibm-icd-rabbitmq/issues). Please note this product is not supported via the IBM Cloud Support Center.", | ||
| "features": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sync features with Redis. Check the order, add links to the features, etc.
ibm_catalog.json
Outdated
| { | ||
| "label": "Standard", | ||
| "name": "standard", | ||
| "label": "Fully Configurable", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be "label": "Fully configurable",
|
|
||
| ############################################################## | ||
| # Encryption | ||
| ############################################################## |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tags rename to resource_tags,sync with Redis
| error_message = "When 'use_ibm_owned_encryption_key' is false, you must provide either 'existing_kms_instance_crn' (to create a new key) or 'existing_kms_key_crn' (to use an existing key)." | ||
| condition = (var.existing_kms_instance_crn == null && var.existing_kms_key_crn == null && var.existing_backup_kms_key_crn == null) || var.kms_encryption_enabled | ||
| error_message = "When either 'existing_kms_instance_crn', 'existing_kms_key_crn' or 'existing_backup_kms_key_crn' is set then 'kms_encryption_enabled' must be set to true." | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
existing_kms_instance_crn sync desc with Redis
| var.existing_secrets_manager_instance_crn != null | ||
| ) | ||
| error_message = "`existing_secrets_manager_instance_crn` is required when adding service credentials to a secrets manager secret." | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
existing_backup_kms_key_crn sync with Redis
| error_message = "`existing_secrets_manager_instance_crn` is required when adding service credentials to a secrets manager secret." | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use_default_backup_encryption_key sync with redis
| error_message = "`existing_secrets_manager_instance_crn` is required when adding service credentials to a secrets manager secret." | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add empty line before variable "provider_visibility". sync with redis
|
CRA scanning failed with
|
tests/pr_test.go
Outdated
| TerraformDir: fullyConfigurableSolutionTerraformDir, | ||
| BestRegionYAMLPath: regionSelectionPath, | ||
| Prefix: "rabbitmq-st-da-upg", | ||
| Prefix: "rmq-da-upg", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still too long. Limit is 16 and the test infrastructure adds 7, so limit is 9 characters.
| "ibmcloud_api_key": $VALIDATION_APIKEY, | ||
| "region": "us-south", | ||
| "tags": $TAGS, | ||
| "name": $PREFIX, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
existing_resource_group_name is a required property. It should be included here for the catalog pipeline.
|
/run pipeline |
akocbek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
about the diagrams:
Please use the latest stencil version. For example, the latest stencil gives you a dotted box for resource group.
resource group has dots, but its not the right box, should be red colored with icon, also other icons are different
also check how the KMS encryption is added in other DAs. For an example the redis

| ], | ||
| "service_name": "Resource group only", | ||
| "notes": "Viewer access is required in the resource group you want to provision in." | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check security-enforced as well
ibm_catalog.json
Outdated
| "title": "KMS encryption", | ||
| "description": "Provides KMS encryption for the data that you store in the database." | ||
| "description": "Provides [KMS encryption](https://cloud.ibm.com/docs/messages-for-rabbitmq?topic=messages-for-rabbitmq-key-protect&interface=ui) for the data that you store in the database, enhancing data security." | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove extra line
| { | ||
| "key": "existing_rabbitmq_instance_crn" | ||
| } | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check security-enforced as well
| "service_name": "kms" | ||
| "service_name": "hs-crypto", | ||
| "notes": "[Optional] Editor access is required to create keys in HPCS. It is only required when using HPCS for encryption." | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
existing_resource_group_name -> delete required
check security-enforced as well
| type = number | ||
| description = "The number of members that are allocated. [Learn more](https://cloud.ibm.com/docs/messages-for-rabbitmq?topic=messages-for-rabbitmq-resources-scaling)." | ||
| default = 3 | ||
| default = 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pipeline failed, it looks it has to be 3 after all. let's update it everywhere back to 3.
…m-icd-rabbitmq into ks-da
|
/run pipeline |
| "key": "cbr_rules" | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that arrow was an accident. I've removed it.
|
/run pipeline |
|
/run pipeline |
|
Only upgrade test is failing now. |
|
@kierramarie upgrade test failed because this is expected, since main branch doesn't have |
|
/run pipeline |
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |




Description
Updated standard solution to fully-configurable and added security enforced da.
Release required?
x.x.X)x.X.x)X.x.x)Release notes content
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:
Checklist for reviewers
For mergers