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

Fix "quicksight" identify type, update cfn caller templates + clearer admin user/dashboard user creation #111

Merged
merged 9 commits into from
Feb 28, 2025

Conversation

georgepstaylor
Copy link
Member

@georgepstaylor georgepstaylor commented Feb 27, 2025

This PR makes some fixes discovered during a new deployment.

  • Fixes using "QUICKSIGHT" as the identity type for a user by adding a try() to the conditional as terraform evaluates all sides before comparing.
  • Updates the "AWS Billing Data Export Aggregation" cfn template to v0.2.0 to fix an issue with the remote stacks requiring new inputs.
  • Changes the conditional logic for creating an admin user. The username and email need to be both provided as well as a flag enable_quicksight_admin in order for an admin user to be created (username/email configurable as before)
  • var.quicksights_username renamed to var.quicksight_dashboard_owner to increase clarity around what this user/input is used for.

The default logic is now that the destination module will create an admin user 'admin' and that same user will be used as the dashboards owner. The local to evaluate whether an admin user should be created or not is removed so that a terraform error is raised when enable_quicksight_admin=true and var.quicksight_admin_email has not be set (due to email being a required attr for the aws_quicksight_user resource. You will be able to stop this module managing the admin user by setting enable_quicksight_admin=false

Copy link

github-actions bot commented Feb 27, 2025

Pull Request Review Status

  • 🖌 Terraform Format and Style: success
  • 🔍 Terraform Linting: success
  • 👮 Terraform Security Check: success
  • 👮 Terraform Security Checkov: success
  • 🔘 Terraform Tests: success
  • 🔧 Terraform Initialisation: success
  • 🤖 Terraform Validation: success
  • 🤖 Terraform Example Validation: success
  • 📖 Terraform Documentation: success
  • 🔖 Commitlint: success

Working Directory: examples/basic
Pusher: @georgepstaylor, Action: pull_request
Workflow Run Link: https://github.com/appvia/terraform-aws-cudos/actions/runs/13589260554

@georgepstaylor georgepstaylor changed the title fix: error accessing empty tuple when not using IAM linked auth Fix quicksight", update cfn caller templates + Feb 28, 2025
@georgepstaylor georgepstaylor changed the title Fix quicksight", update cfn caller templates + Fix "quicksight" identify type, update cfn caller templates + clearer admin user/dashboard user creation Feb 28, 2025
@georgepstaylor georgepstaylor force-pushed the fix/non-iam-auth-error branch 2 times, most recently from 7950dfd to 81ee920 Compare February 28, 2025 12:48
@georgepstaylor georgepstaylor force-pushed the fix/non-iam-auth-error branch 2 times, most recently from 98125a7 to 29cdcf0 Compare February 28, 2025 13:23
@gambol99
Copy link
Member

LGTM .. thank you kindly 🫡

@georgepstaylor georgepstaylor merged commit 82686a0 into main Feb 28, 2025
12 checks passed
@georgepstaylor georgepstaylor deleted the fix/non-iam-auth-error branch February 28, 2025 16:27
@georgepstaylor georgepstaylor restored the fix/non-iam-auth-error branch February 28, 2025 17:14
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