-
Notifications
You must be signed in to change notification settings - Fork 2
feat: scc workload protection DA #181
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
|
/run pipeline |
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.
Can you add the ibm_catalog.json and .catalog-onboard-pipeline.yaml files please? Also I think we should name the variation Baseline instead of Standard as it looks like this is the naming convention we are going it. In parallel I am trying to get confirmation n that
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.
A few more comments. Also when creating the ibm_catalog.json make sure to follow all the best practises we are rolling out as per https://github.ibm.com/GoldenEye/issues/issues/12542. There is a reference PR which you can copy the approach in (its still under review, but you can see the review comments)
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.
See latest comments.
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.
@jor2 still several things to address. See comments.
Also we should not be using display_name in the catalog json - please remove them.
In the diagram update Monitoring -> Existing Monitoring instance and on the arrow, put "metrics" on it
tests/pr_test.go
Outdated
| options := setupOptions(t, "scc-wp-upg", advancedExampleDir) | ||
| var region = validRegions[rand.IntN(len(validRegions))] | ||
|
|
||
| options := testhelper.TestOptionsDefault(&testhelper.TestOptions{ |
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.
Can we use the new schematics upgrade test instead? All DA tests should use schematics
tests/pr_test.go
Outdated
| const advancedExampleDir = "examples/advanced" | ||
| const basicExampleDir = "examples/basic" | ||
| const resourceGroup = "geretain-test-resources" | ||
| const standardSolutionDir = "solutions/standard" |
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 should be solutions/fully-configurable
tests/other_tests.go
Outdated
| assert.NotNil(t, output, "Expected some output") | ||
| } | ||
|
|
||
| func TestRunAdvancedUpgradeExample(t *testing.T) { |
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 should be a normal test - not upgrade test
| variable "scc_workload_protection_instance_name" { | ||
| description = "The name for the Workload Protection instance that is created by this solution. Must begin with a letter. Applies only if `provision_scc_workload_protection` is true. If a prefix input variable is specified, the prefix is added to the name in the `<prefix>-<name>` format." | ||
| type = string | ||
| default = "workload_protection" |
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.
scc-workload-protection
| ], | ||
| "short_description": "Creates and configures IBM Security and Compliance Center resources", | ||
| "long_description": "This architecture supports creating and configuring IBM Security and Compliance Center resources.", | ||
| "offering_docs_url": "https://github.com/terraform-ibm-modules/terraform-ibm-scc-workload-protection/blob/main/solutions/instances/README.md", |
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.
wrong directory
ibm_catalog.json
Outdated
| "solution" | ||
| ], | ||
| "short_description": "Creates and configures IBM Security and Compliance Center resources", | ||
| "long_description": "This architecture supports creating and configuring IBM Security and Compliance Center resources.", |
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.
needs to be updated
ibm_catalog.json
Outdated
| "terraform", | ||
| "solution" | ||
| ], | ||
| "short_description": "Creates and configures IBM Security and Compliance Center resources", |
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.
needs to be updated
ibm_catalog.json
Outdated
| { | ||
| "products": [ | ||
| { | ||
| "name": "deploy-arch-ibm-workload-protection", |
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.
maybe go with deploy-arch-ibm-scc-workload-protection
.catalog-onboard-pipeline.yaml
Outdated
| --- | ||
| apiVersion: v1 | ||
| offerings: | ||
| - name: deploy-arch-ibm-workload-protection |
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.
maybe go with deploy-arch-ibm-scc-workload-protection
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.
Final few comments
| variable "region" { | ||
| type = string | ||
| default = "us-south" | ||
| description = "The region to provision Security and Compliance Center resources 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.
@jor2 still needs to be addressed
|
|
||
| This solution supports provisioning and configuring the following infrastructure: | ||
|
|
||
| - A resource group, if one is not passed 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.
It does not create a new RG
ibm_catalog.json
Outdated
| "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 that repository [https://github.com/terraform-ibm-modules/terraform-ibm-scc-workload-protection/issues](https://github.com/terraform-ibm-modules/terraform-ibm-scc-workload-protection/issues). Please note this product is not supported via the IBM Cloud Support Center.", | ||
| "flavors": [ | ||
| { | ||
| "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.
Fully configurable
.catalog-onboard-pipeline.yaml
Outdated
| offerings: | ||
| - name: deploy-arch-ibm-scc-workload-protection | ||
| kind: solution | ||
| catalog_id: _ |
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.
7df1e4ca-d54c-4fd0-82ce-3d13247308cd
.catalog-onboard-pipeline.yaml
Outdated
| - name: deploy-arch-ibm-scc-workload-protection | ||
| kind: solution | ||
| catalog_id: _ | ||
| offering_id: _ |
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.
4322cf44-2289-49aa-a719-dd79e39b14dc
tests/pr_test.go
Outdated
| const advancedExampleDir = "examples/advanced" | ||
| const basicExampleDir = "examples/basic" | ||
| const resourceGroup = "geretain-test-resources" | ||
| const standardSolutionDir = "solutions/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.
standardSolutionDir -> fullyConfigurableDADir
|
/run pipeline |
|
🎉 This PR is included in version 1.5.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Description
Currently the SCC Workload Protection service is provisioned (optionally) as part of the SCC DA.
We want to split it into its own DA. The code should live in the module repo: https://github.com/terraform-ibm-modules/terraform-ibm-scc-workload-protection
After doing this, we should remove support for Workload Protections from the SCC DA, and instead leverage add-ons to make it an optional addon.
We might also want to consider moving the SCC DA code from terraform-ibm-scc-da into the terraform-ibm-scc and remove the scc-da specific repo.
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