Add Managed Grafana workspace to dev - #2267
Closed
theseanything wants to merge 2 commits into
Closed
Conversation
Adds an AWS Managed Grafana workspace (AWS_SSO, service-managed, CloudWatch/X-Ray/Athena data sources, Grafana 12.4) to the health root behind an environmental_settings flag enabled only in dev, plus the deployer permissions needed to create it.
There was a problem hiding this comment.
🟡 Changes recommended
The current output and deployer permissions are likely to break non-dev applies and/or Terraform refresh due to an unsafe one([]) output and missing Grafana Describe/ListTags IAM actions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces an optional AWS Managed Grafana workspace under the forms/health Terraform deployment, controlled by a new environmental_settings.enable_managed_grafana flag (enabled in dev). It also updates the deployer IAM policy to allow provisioning and managing the workspace and its associated IAM role.
Changes:
- Add
enable_managed_grafanatoenvironmental_settingsinputs and enable it in dev tfvars. - Provision an IAM role plus an AWS Managed Grafana workspace (AWS SSO auth, service-managed permissions, Athena/CloudWatch/X-Ray data sources) in
forms/healthwhen enabled. - Extend the deployer policy with Grafana/Sso/Organizations permissions and allowlisting for the new workspace role.
File summaries
| File | Description |
|---|---|
| infra/modules/deployer-access/policy.tf | Adds Grafana and SSO/Organizations permissions and allowlists the new Grafana workspace IAM role for IAM management/passrole. |
| infra/deployments/forms/tfvars/dev.tfvars | Enables the managed Grafana feature flag in dev. |
| infra/deployments/forms/inputs.tf | Adds enable_managed_grafana as an optional environmental setting input. |
| infra/deployments/forms/health/outputs.tf | Exposes the Grafana workspace endpoint as an output. |
| infra/deployments/forms/health/grafana.tf | Adds conditional creation of the Grafana IAM role, managed policy attachments, and workspace resource. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1
to
+3
| output "grafana_workspace_endpoint" { | ||
| value = one(aws_grafana_workspace.this[*].endpoint) | ||
| } |
Comment on lines
+471
to
+477
| actions = [ | ||
| "grafana:DeleteWorkspace", | ||
| "grafana:UpdateWorkspace", | ||
| "grafana:UpdateWorkspaceConfiguration", | ||
| "grafana:TagResource", | ||
| "grafana:UntagResource" | ||
| ] |
There was a problem hiding this comment.
🟡 Changes recommended
The current Grafana output expression will fail in non-dev envs when Grafana is disabled, and the Glue catalog resource uses an invalid boolean value type that is likely to break Terraform plan/apply.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
infra/deployments/forms/health/outputs.tf:3
- This output will fail to plan/apply in environments where Managed Grafana is disabled (the default): when the workspace resource count is 0,
one([])raises an error. Make the output conditional (or usetry) so non-dev environments can still apply.
output "grafana_workspace_endpoint" {
value = one(aws_grafana_workspace.this[*].endpoint)
}
infra/modules/submission-events/glue-catalog.tf:30
allow_full_table_external_data_accessis set to the string "True"; Terraform arguments here are typed, and this should be a boolean literal. As written, this is likely to fail type checking during plan/apply.
}
allow_full_table_external_data_access = "True"
}
- Files reviewed: 21/21 changed files
- Comments generated: 2
- Review effort level: Lite
Comment on lines
+1
to
+13
| # Integrates S3 Tables with AWS analytics services (Athena, Firehose, etc.) by | ||
| # federating all S3 table buckets in the account into the Glue Data Catalog. | ||
| # This is an account+region singleton: the name must be "s3tablescatalog" and | ||
| # it covers every table bucket in the account, not just the one in this module. | ||
| # Access control is IAM-based (IAM_ALLOWED_PRINCIPALS), so consumers need IAM | ||
| # permissions only, no Lake Formation grants. | ||
| resource "aws_glue_catalog" "s3tablescatalog" { | ||
| name = "s3tablescatalog" | ||
|
|
||
| federated_catalog { | ||
| connection_name = "aws:s3tables" | ||
| identifier = "arn:aws:s3tables:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:bucket/*" | ||
| } |
Comment on lines
+1
to
+5
| module "submission_events" { | ||
| count = var.forms_runner_settings.enable_submission_events_analytics ? 1 : 0 | ||
| source = "../../../modules/submission-events" | ||
| env_name = var.environment_name | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds an AWS Managed Grafana workspace to the
forms/healthroot, gated by a newenvironmental_settings.enable_managed_grafanaflag that is onlytruein dev.AWS_SSOauth,SERVICE_MANAGED, data sourcesATHENA/CLOUDWATCH/XRAY, Grafana12.4<env>-grafana-workspacewith the AWS managed Grafana CloudWatch/Athena and X-Ray read-only policiesgrafana:/sso:actions and the new role in its allowlistAthena is permissions-only for now (no workgroup or results bucket). Users are assigned to the workspace via Identity Center in the console.
Apply
forms/accountin dev before the pipeline reachesforms/health.