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.

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

sap_vm_provision: feat: aws security enhancements #4

Merged
merged 5 commits into from
Apr 24, 2024

Conversation

marcelmamula
Copy link
Contributor

@marcelmamula marcelmamula commented Mar 13, 2024

PR covers following enhancements:
#1

  • no_log: "{{ __sap_vm_provision_no_log }}" to hide secrets in -vvv debug mode
  • access_key and secret_key were added directly into aws modules because of point above
  • region was not added into modules, because Route53 module requires extra parameter interpreter. Region is kept as environment variable.

Additionally covered:

  • S-User password assignment in var task is hidden with no_log: true
  • Platform guidance was updated with detailed breakdown of IAM Actions as additional option for IAM Policy setup to enhance security.

Rescue block added for failed awscli calls.

@sean-freeman
Copy link
Member

sean-freeman commented Apr 13, 2024

@marcelmamula the change for AWS Route53 DNS Record becomes a destructive action, therefore we will need to implement a new default variable sap_vm_provision_aws_private_dns_overwrite: false to this PR (use common var name pattern _private_dns because new vars have been requested for MS Azure Private DNS also).

Changes to Env Var approach to avoid use of no_log at the Ansible Task Block level, will require re-work across all target Infrastructure Platforms and re-test for all. This will take some time, only then can I accept PR.

@sean-freeman sean-freeman changed the title feat: aws security enhancements sapfeat: aws security enhancements Apr 18, 2024
@sean-freeman sean-freeman changed the title sapfeat: aws security enhancements sap_vm_provision: feat: aws security enhancements Apr 18, 2024
@sean-freeman
Copy link
Member

sean-freeman commented Apr 20, 2024

@marcelmamula overwrite is only added to the provisioned hosts DNS Record, it needs to be added to all amazon.aws.route53 Ansible Modules (including the commented-out code) in execute_setup_ha.yml file also

EDIT: On second thoughts, please remove overwrite from this PR. Continue discussion in #2

@sean-freeman
Copy link
Member

As discussed separately and agreed, this PR contains - the hidden debugging var added for no_log, rename of task registered vars, remove environment var and declare secrets as module args, and a new rescue block that will output errors (that no_log will otherwise hide). This open PR merges these changes first, and I will append another PR for the other infrastructure platforms.

lgtm 👍 Merge when ready

@marcelmamula marcelmamula merged commit e360e3b into sap-linuxlab:dev Apr 24, 2024
1 of 3 checks passed
@marcelmamula marcelmamula deleted the aws-fixes branch April 30, 2024 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants