-
Notifications
You must be signed in to change notification settings - Fork 3
Rally #350
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
ocofaigh
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.
Diagram feedback:
- [Optional] Resource Group -> Resource group
- Replace "s2s IAM auth" with "IAM credentials engine"
- Should Event Notifications be in its own box (like you have for KMS). And in the box it should indicate that a topic is created in the EN instance
- The access group icon is no longer showing
@rajatagarwal-ibm Can you also review the points in https://github.ibm.com/GoldenEye/issues/issues/13599 to ensure that are all met too - thanks
ocofaigh
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.
@rajatagarwal-ibm Can you please expose the 2 Observability virtual inputs in this PR please? Same change as terraform-ibm-modules/terraform-ibm-event-notifications#482
ocofaigh
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.
Missing the changes in https://github.ibm.com/GoldenEye/issues/issues/13599 - point 7 and 8
ibm_catalog.json
Outdated
| }, | ||
| { | ||
| "key": "logs_routing_tenant_regions", | ||
| "type": "array", |
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 is a bug we need to workaround here. please change to:
| "type": "array", | |
| "type": "list(object)", |
ibm_catalog.json
Outdated
| }, | ||
| { | ||
| "key": "enable_platform_metrics", | ||
| "type": "boolean", |
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.
another workaround needed for this one:
| "type": "boolean", | |
| "type": "bool", |
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.
- The diagram should not be saying "Existing.." anywhere as the addons will deploy these services
- IAM engine is done via an s2s auth policy now. You should make that clear. If your keeping the IAM Engine as a box inside Secrets Manager box, it should have an arrow pointing to The IAM box with the text "s2s auth policy"
- You accidentally committed
solutions/security-enforced/terraform.log - Cleanup required to
DA-cbr_rules.md:- Give more information on what the sample CBR rule value is doing
- Remove any hard coded values, and replace them with
<REPLACE_ME> - Fix the indentation
- No need for double quotes for name and value
|
/run pipeline |
|
/run pipeline |
Re-running.. |
|
/run pipeline |
|
🎉 This PR is included in version 2.6.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |



Description
private_endpoint_typein the provider.tfRelease 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