Skip to content
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.

Sign up for GitHub

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

fix: add depends for cos #74

Merged
merged 7 commits into from
Apr 15, 2024
Merged

fix: add depends for cos #74

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e0a694b
fix: add depends
Apr 14, 2024
35976ee
fix: add depends
Apr 14, 2024
03da8f5
fix: add depends
Apr 14, 2024
b5a699b
fix: add depends
Apr 14, 2024
e0c0792
fix: update order
Apr 15, 2024
4db3ffb
fix: update order
Apr 15, 2024
affa539
fix: update order
Apr 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ No modules.
| [ibm_resource_instance.scc_instance](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/resources/resource_instance) | resource |
| [ibm_scc_instance_settings.scc_instance_settings](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/resources/scc_instance_settings) | resource |
| [ibm_scc_provider_type_instance.scc_provider_type_instance_instance](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/resources/scc_provider_type_instance) | resource |
| [time_sleep.wait_for_authorization_policy](https://registry.terraform.io/providers/hashicorp/time/latest/docs/resources/sleep) | resource |
| [time_sleep.wait_for_scc_cos_authorization_policy](https://registry.terraform.io/providers/hashicorp/time/latest/docs/resources/sleep) | resource |
| [time_sleep.wait_for_scc_wp_authorization_policy](https://registry.terraform.io/providers/hashicorp/time/latest/docs/resources/sleep) | resource |
| [ibm_iam_account_settings.iam_account_settings](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/data-sources/iam_account_settings) | data source |
| [ibm_scc_provider_types.scc_provider_types](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/data-sources/scc_provider_types) | data source |

Expand Down
34 changes: 21 additions & 13 deletions main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ locals {
}

resource "ibm_scc_provider_type_instance" "scc_provider_type_instance_instance" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you leave a code comment above this resource block to explain what its doing (aka attaching an SCC Workload Protection instance)

depends_on = [time_sleep.wait_for_authorization_policy]
depends_on = [time_sleep.wait_for_scc_wp_authorization_policy]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this depends on the time_sleep which depends on the auth policy then I expect to see those resource blocks above this one in the code. Like I said try to keep the dependency flow the same in the code as the dependncy tree order itself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should actually also depend_on ibm_scc_instance_settings.scc_instance_settings because the COS integration must be done before you can use the SCC instance (which means the ibm_scc_instance_settings resource block should also be above this in the code)

count = var.attach_wp_to_scc_instance ? 1 : 0
instance_id = ibm_resource_instance.scc_instance.guid
attributes = { "wp_crn" : var.wp_instance_crn }
Expand All @@ -37,7 +37,7 @@ resource "ibm_scc_provider_type_instance" "scc_provider_type_instance_instance"
}

# workaround for https://github.com/IBM-Cloud/terraform-provider-ibm/issues/4478
resource "time_sleep" "wait_for_authorization_policy" {
resource "time_sleep" "wait_for_scc_wp_authorization_policy" {
depends_on = [ibm_iam_authorization_policy.scc_wp_s2s_access]

create_duration = "30s"
Expand Down Expand Up @@ -71,6 +71,25 @@ resource "ibm_iam_authorization_policy" "scc_wp_s2s_access" {
data "ibm_iam_account_settings" "iam_account_settings" {
}

resource "ibm_scc_instance_settings" "scc_instance_settings" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you leave a code comment above this resource block to explain what its doing (aka attaching a COS bucket and an event notifications instance)

depends_on = [time_sleep.wait_for_scc_cos_authorization_policy]
instance_id = resource.ibm_resource_instance.scc_instance.guid
event_notifications {
instance_crn = var.en_instance_crn
}
object_storage {
instance_crn = var.cos_instance_crn
bucket = var.cos_bucket
}
}

# workaround for https://github.com/IBM-Cloud/terraform-provider-ibm/issues/4478
resource "time_sleep" "wait_for_scc_cos_authorization_policy" {
depends_on = [ibm_iam_authorization_policy.scc_cos_s2s_access]

create_duration = "30s"
}

resource "ibm_iam_authorization_policy" "scc_cos_s2s_access" {
count = var.skip_cos_iam_authorization_policy ? 0 : 1
source_service_name = "compliance"
Expand All @@ -96,17 +115,6 @@ resource "ibm_iam_authorization_policy" "scc_cos_s2s_access" {
}
}

resource "ibm_scc_instance_settings" "scc_instance_settings" {
instance_id = resource.ibm_resource_instance.scc_instance.guid
event_notifications {
instance_crn = var.en_instance_crn
}
object_storage {
instance_crn = var.cos_instance_crn
bucket = var.cos_bucket
}
}

locals {
cos_instance_guid = var.cos_instance_crn != null ? element(split(":", var.cos_instance_crn), length(split(":", var.cos_instance_crn)) - 3) : null
wp_instance_guid = var.wp_instance_crn != null ? element(split(":", var.wp_instance_crn), length(split(":", var.wp_instance_crn)) - 3) : null
Expand Down
4 changes: 4 additions & 0 deletions moved.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
moved {
from = time_sleep.wait_for_authorization_policy
to = time_sleep.wait_for_scc_wp_authorization_policy
}