aws-solutions / aws-control-tower-customizations

The Customizations for AWS Control Tower solution combines AWS Control Tower and other highly-available, trusted AWS services to help customers more quickly set up a secure, multi-account AWS environment using AWS best practices.
https://docs.aws.amazon.com/controltower/latest/userguide/cfct-overview.html
Apache License 2.0
355 stars 205 forks source link

Different logic of processing the non-existent organizational_units in `stack_set` and `scp` deployment_targets types #126

Closed suankan closed 7 months ago

suankan commented 1 year ago

Describe the bug Currently I can specify non-existent OUs in my custom resource when using deploy_method: stack_set:

  - name: custom-resource-01
    deploy_method: stack_set
    resource_file: templates/custom-resource-01.yaml
    deployment_targets:
      organizational_units:
        - Nonexistent # This one does not actually exist
        - Security

The CodeBuild job processes this gracefully by simply omitting the non-existent OU. And this is a desired behaviour because I'm deploying Dev and Prod landing zones from one single manifest.yaml where I wanted to just list Dev and Prod OUs.

However the CodeBuild job stumbles if I try the same when using deploy_method: scp:

  - name: custom-policy-01
    deploy_method: scp
    resource_file: templates/custom-policy-01.yaml
    deployment_targets:
      organizational_units:
        - Nonexistent # This one does not actually exist
        - Security

I'm getting this error in my CodeBuild:

{"time_stamp": "2022-07-18 18:51:35,126","log_level": "INFO","log_message": [manifest_parser.get_ou_id] Organizations Root Id: r-****}
268 |  
269 | {"time_stamp": "2022-07-18 18:51:35,126","log_level": "INFO","log_message": [manifest_parser.get_ou_id] Looking up the OU Id for OUName: Nonexistent with nested ou delimiter: ':'}
270 |  
271 | {"time_stamp": "2022-07-18 18:51:35,457","log_level": "INFO","log_message": [manifest_parser._get_ou_id] _list_ou_for_parent response: None}
272 |  
273 | {"time_stamp": "2022-07-18 18:51:35,457","log_level": "ERROR","log_message": Unhandled Exception: OU id is not found for Nonexistent}
274 | Traceback (most recent call last):
275 | File "state_machine_trigger.py", line 59, in main
276 | sm_input_list = get_scp_inputs()
277 | File "state_machine_trigger.py", line 87, in get_scp_inputs
278 | return parse.scp_manifest()
279 | File "/codebuild/output/src670528028/src/cfct/manifest/manifest_parser.py", line 46, in scp_manifest
280 | return get_scp_input.parse_scp_manifest_v2()
281 | File "/codebuild/output/src670528028/src/cfct/manifest/manifest_parser.py", line 133, in parse_scp_manifest_v2
282 | final_ou_list = org_data.get_final_ou_list(attach_ou_list)
283 | File "/codebuild/output/src670528028/src/cfct/manifest/manifest_parser.py", line 664, in get_final_ou_list
284 | ou_id= self.get_ou_id(ou_name, ":")
285 | File "/codebuild/output/src670528028/src/cfct/manifest/manifest_parser.py", line 688, in get_ou_id
286 | raise ValueError("OU id is not found for {}".format(nested_ou_name))
287 | ValueError: OU id is not found for Nonexistent
288 | Traceback (most recent call last):
289 | File "state_machine_trigger.py", line 116, in <module>
290 | main()
291 | File "state_machine_trigger.py", line 59, in main
292 | sm_input_list = get_scp_inputs()
293 | File "state_machine_trigger.py", line 87, in get_scp_inputs
294 | return parse.scp_manifest()
295 | File "/codebuild/output/src670528028/src/cfct/manifest/manifest_parser.py", line 46, in scp_manifest
296 | return get_scp_input.parse_scp_manifest_v2()
297 | File "/codebuild/output/src670528028/src/cfct/manifest/manifest_parser.py", line 133, in parse_scp_manifest_v2
298 | final_ou_list = org_data.get_final_ou_list(attach_ou_list)
299 | File "/codebuild/output/src670528028/src/cfct/manifest/manifest_parser.py", line 664, in get_final_ou_list
300 | ou_id= self.get_ou_id(ou_name, ":")
301 | File "/codebuild/output/src670528028/src/cfct/manifest/manifest_parser.py", line 688, in get_ou_id
302 | raise ValueError("OU id is not found for {}".format(nested_ou_name))
303 | ValueError: OU id is not found for Nonexistent
304 |  
305 | [Container] 2022/07/18 18:51:35 Command did not exit successfully bash execute_stage_scripts.sh $STAGE_NAME $LOG_LEVEL $WAIT_TIME $SM_ARN $ARTIFACT_BUCKET $KMS_KEY_ALIAS_NAME $BOOL_VALUES $NONE_TYPE_VALUES exit status 1
306 | [Container] 2022/07/18 18:51:35 Phase complete: BUILD State: FAILED
307 | [Container] 2022/07/18 18:51:35 Phase context status code: COMMAND_EXECUTION_ERROR Message: Error while executing command: bash execute_stage_scripts.sh $STAGE_NAME $LOG_LEVEL $WAIT_TIME $SM_ARN $ARTIFACT_BUCKET $KMS_KEY_ALIAS_NAME $BOOL_VALUES $NONE_TYPE_VALUES. Reason: exit status 1

To Reproduce Try to deploy a custom resource and scp policy using the above code examples.

Expected behavior

CodeBuild jobs for both deployment_method stack_set and scp should behave identically when non-existent OUs are found in deployment_targets. E.g. either

Please complete the following information about the solution:

To get the version of the solution, you can look at the description of the created CloudFormation stack. For example, "(SO0089) - customizations-for-aws-control-tower Solution. Version: v1.0.0". You can also find the version from releases

Screenshots n/a

Additional context n/a

balltrev commented 1 year ago

Thank you for bringing this to our attention @suankan. I've gone ahead and made a backlog with the team to align the behavior here.

suankan commented 1 year ago

Hi @balltrev ,

I created a quick fix which might be good to review and include: https://github.com/aws-solutions/aws-control-tower-customizations/pull/127

gabrielbac commented 1 year ago

This is a bug that should be prioritized.

stumins commented 7 months ago

Hi @suankan @gabrielbac,

We just released CFCT v2.7.0, which includes this fix. Non-existent OUs are now ignored during SCP provisioning, aligning the behavior with how non-existent OUs are treated during StackSet provisioning.