-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: update module inputs for da solution #53
Conversation
/run pipeline |
/run pipeline |
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 comments
solutions/instances/variables.tf
Outdated
@@ -202,6 +202,18 @@ variable "scc_instance_tags" { | |||
default = [] | |||
} | |||
|
|||
variable "attach_wp_to_scc_instance" { |
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.
don't expose this - if provision_scc_workload_protection
is true then the DA code should automatically set attach it to the SCC instance
tests/pr_test.go
Outdated
@@ -79,6 +79,8 @@ func TestInstancesInSchematics(t *testing.T) { | |||
{Name: "scc_cos_bucket_access_tags", Value: permanentResources["accessTags"], DataType: "list(string)"}, | |||
{Name: "scc_wp_access_tags", Value: permanentResources["accessTags"], DataType: "list(string)"}, | |||
{Name: "cos_instance_access_tags", Value: permanentResources["accessTags"], DataType: "list(string)"}, | |||
{Name: "attach_wp_to_scc_instance", Value: true, DataType: "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.
remove this since we will remove the attach_wp_to_scc_instance
variable
/run pipeline |
/run pipeline |
tests/pr_test.go
Outdated
@@ -79,6 +79,8 @@ func TestInstancesInSchematics(t *testing.T) { | |||
{Name: "scc_cos_bucket_access_tags", Value: permanentResources["accessTags"], DataType: "list(string)"}, | |||
{Name: "scc_wp_access_tags", Value: permanentResources["accessTags"], DataType: "list(string)"}, | |||
{Name: "cos_instance_access_tags", Value: permanentResources["accessTags"], DataType: "list(string)"}, | |||
{Name: "provision_scc_workload_protection", Value: true, DataType: "bool"}, | |||
{Name: "skip_scc_wp_auth_policy", Value: false, DataType: "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.
these are the default values so no need to explicitly specify here
solutions/instances/variables.tf
Outdated
variable "skip_scc_wp_auth_policy" { | ||
type = bool | ||
default = false | ||
description = "Set to true to skip the creation of an IAM authorization policy that permits the SCC instance created by this solution read access to the workload protection instance. Only used if `attach_wp_to_scc_instance` is 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.
remove attach_wp_to_scc_instance
from the description - its no an exposed variable in this DA. I guess it should be replaced by provision_scc_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.
see comments
/run pipeline |
🎉 This PR is included in version 1.4.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Description
Update module inputs in da solution to attach workload protection instance.
#23
Release required?
x.x.X
)x.X.x
)X.x.x
)Release notes content
Update module inputs in da solution to attach workload protection instance.
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