Add Terraform infrastructure for managed OpenSearch web analytics pipeline - #77
Add Terraform infrastructure for managed OpenSearch web analytics pipeline#77jordanpadams wants to merge 127 commits into
Conversation
Deploys the full web analytics infrastructure: managed OpenSearch domain (VPC-only, FGAC via IAM, OpenSearch 2.19), Logstash EC2, S3 log bucket, and IAM policies. Splits admin-only operations (IAM policy creation, EC2 launch) into dedicated tasks. Removes hardcoded sensitive values (SSO role ARN, instance profile name) in favor of variables. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ntain permissions' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Rename pds_resource_prefix -> s3_bucket_prefix (S3 only) and ec2_name_prefix/policy_name_prefix -> resource_prefix (all other resources). Removes gh01dc from IAM policy and EC2 resource names. Add dev.tfvars.example files for all modules. Fix YAML colon quoting in Taskfile aliases and deps. Run terraform fmt across all modules. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Automates first-boot setup: clones repo, deploys Logstash pipeline config, applies OpenSearch ECS index template, and starts Logstash. Can also be run manually via SSM on existing instances. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes stale Chunk 1/2/3 references, adds logstash-init.sh step, clarifies permission requirements per step, updates teardown and architecture notes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
FGAC (fine-grained access control) required mapping IAM roles to backend roles via the OpenSearch security API after every domain deployment. Since access is controlled entirely by the IAM resource-based policy (which allows the EC2 instance role and mcp-tenantOperator), FGAC provides no additional security benefit and only adds operational friction. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
tenantOperator roles are no longer applicable. EC2 instance role is the only principal that needs access to S3 (for log ingestion) and OpenSearch (for writing indexed data). Also collapses the two-statement OpenSearch policy into a single AllowEC2Role statement. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…example
The {venue}-en-platform-engineer role only exists in test/prod, not dev.
AWS rejects resource-based policies that reference non-existent principals,
so the AllowPlatformEngineer statement must only be included when the role
exists in the target account.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Introduces tfvars/common.tfvars (aws_region, partition, tenant, component, cicd, managedby) shared by all three Terraform root modules. Each module's venue-specific tfvars now only contains venue + module-specific values. Taskfile updated to pass -var-file=.../tfvars/common.tfvars before each module's own tfvars. Adds missing prod.tfvars.example files for root and IAM modules. Removes opensearch_master_user_arn from OpenSearch tfvars now that FGAC is disabled. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Clearly marks Steps 3 (IAM) and 4 (EC2) as admin-only with required permissions. Adds unset AWS_PROFILE note for Terraform S3 backend compatibility. Updates teardown section with admin markers. Documents OpenSearch access policy and platform-engineer role pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
OpenSearch module does not declare a partition variable, so passing it via common.tfvars produced a warning. partition is now declared only in the root and IAM module tfvars where it is actually used. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ss root and IAM modules
partition, s3_bucket_prefix, resource_prefix, ec2_role_name, and opensearch_domain_name
were duplicated between terraform/tfvars/ and terraform/iam/policies/tfvars/. These values
are shared between root and IAM modules but differ per venue, so they can't go in the global
common.tfvars (which would cause undeclared-variable warnings in opensearch_managed).
Solution: common-infra-<venue>.tfvars, passed to root and IAM tasks via {{.VENUE}} in
Taskfile. Module-specific venue files now only contain what's unique to that module.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…+ IAM
Replaces the two-file pattern (common.tfvars + common-infra-{venue}.tfvars) with
a single common-{venue}.tfvars containing both global and infra-specific values.
Opensearch tasks remain unaffected — they use only their own per-venue tfvars.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace concrete identifiers (bucket names, role names, VPC IDs, SG names, domain names) with empty strings or generic descriptions. Examples should show structure, not real values. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
opensearch_managed uses only its own per-venue tfvars — common-{venue}.tfvars
is for root and IAM modules only (opensearch doesn't declare those variables).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Smoke test script checks S3 access, OpenSearch reachability, and OpenSearch SigV4 auth from the EC2 — all via SSM parameter lookups, no hardcoded values - Publish S3 bucket name to SSM at /pds/web-analytics/s3/bucket_name so the smoke test and other tooling can look it up without hardcoding - Add s3_bucket_name SSM parameter target to s3:plan/s3:deploy Taskfile tasks Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…bility sudo pip installs to root's home by default on AL2023; --break-system-packages installs to the system site-packages so all users can import boto3. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Buckets failure mode An empty S3_CF_BUCKET_NAME templates into the EN CloudFront pipeline as bucket => "", which fails at Logstash runtime with a misleading s3:ListAllMyBuckets AccessDenied error instead of a clear 'not configured' message -- IAM is working correctly, the bucket name is just missing. logstash-deploy.sh now falls back to the previously-deployed value (read from /etc/logstash/env) instead of silently resetting to empty when the var isn't passed, and warns loudly at deploy time if it's still empty. Documented the failure mode in README.md Troubleshooting and made the first-time manual install instructions require an explicit value (real bucket or deliberate empty string) rather than an easy-to-skip placeholder. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
aws ssm start-session lands as root regardless of --document-name/Run-As in practice (that depends on an IAM grant on the document ARN that may not be provisioned), so every connect instruction now says explicitly to switch with `sudo runuser -l logstash` before doing anything. Adds a "Connecting to the Logstash EC2 (SSM)" section to CLAUDE.md covering this, and removes a stale reference to the old log-fix branch. Also finishes a partial fix (found already in progress in README.md) that replaces journalctl --user-unit logstash with tail -f /var/log/logstash/logstash-plain.log throughout scripts/smoke-test.sh and terraform/README.md — journalctl requires adm/wheel group membership the logstash user doesn't have. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Matches the convention in pdc-cds-infra/terraform/cloudfront/pds-main: all data blocks (aws_caller_identity, aws_ssm_parameter reads, aws_iam_policy_document, and the AMI/SG/subnet lookups) live in a dedicated data.tf, separate from resource/locals files. - terraform/logstash: data.tf gets aws_caller_identity, both SSM parameter reads, and the three manage_ec2_instance-gated lookups; main.tf keeps only locals and resources. - terraform/s3: moves the aws_caller_identity that was stray in provider.tf into data.tf. - terraform/iam/policies/web-analytics: moves aws_caller_identity, the opensearch_arn SSM read, and the aws_iam_policy_document (itself a data source) into data.tf; main.tf is now just the aws_iam_policy resource. The root terraform/iam/policies wrapper had no data blocks to move. Also includes an already-pending logstash_version bump (8.17.0 -> 8.18.0) in terraform/logstash/variables.tf that predated this change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 91 out of 102 changed files in this pull request and generated no new comments.
Files not reviewed (3)
- terraform/iam/policies/.terraform.lock.hcl: Generated file
- terraform/logstash/.terraform.lock.hcl: Generated file
- terraform/s3/.terraform.lock.hcl: Generated file
Suppressed comments (5)
scripts/logstash-deploy.sh:126
- This deploy script relies on
aws configure export-credentials, which only exports credentials from an AWS CLI profile and typically fails on EC2 instances using instance-role (IMDS) credentials. Withset -u, a missing AWS_SESSION_TOKEN will also abort the script when building the curl headers. Prefer exporting credentials from boto3/botocore so the template apply works reliably on the instance role.
src/pds/web_analytics/egress_report.py:162 - The OpenSearch aggregations sum
http.response.body.bytes, but the email renders those raw byte sums as “GB”. This will significantly overstate/understate values depending on how the response is interpreted. Convert bytes → GB when rendering. Also, bucket keys come from indexed data and should be HTML-escaped before embedding in the email body to avoid HTML/script injection in reports.
src/pds/web_analytics/egress_report.py:133 - If no AWS credentials are available (e.g., running locally without a profile/role), boto3.Session().get_credentials() returns None and SigV4Auth will raise an unhelpful exception. Add an explicit check with a clear error so failures are actionable.
tests/test_egress_report.py:26 - These fixture values represent the raw aggregation output. If the egress report converts byte sums to GB (as the query suggests by summing a
*.bytesfield), the sample response should use byte values so the expected rendered “123.46 GB” remains correct.
config/logstash/config/shared/pds-filter.conf:729 - This geoip guard checks for
_grokparsefailure, but the main grok filter uses a customtag_on_failurevalue (_grok_filter_initial_parse_failure), so grok parse failures won’t be excluded by this condition. That means geoip/DNS enrichment can run on malformed events, and downstream output routing that relies on grok-failure tags may not behave as intended. Consider normalizing the grok-failure tag here (or aligning the grok tag_on_failure) so failures are consistently detected.
# Determine the geographic location from which the request came.
# Skip only for records that failed parsing — informational tags like
# _url_query_parse_skip or _empty_user_agent don't indicate bad records.
if "bad_log" not in [tags] and "_grokparsefailure" not in [tags] and "_missing_source_address" not in [tags] and [source][address] {
geoip {
…params number_of_replicas was 1 -- unallocatable on a single-data-node dev domain, causing yellow cluster health on every index. Set to 0 (matches AWS's dataNodes-1 guidance for this venue). Added a path-scoped dynamic template forcing every field under url.query_params.* to keyword, regardless of inferred value type. This is the same guarantee `flattened` was meant to provide (no per-key type inference, so no cross-index type conflicts on arbitrary query-string params) without needing to revisit why the flattened mapping itself failed to apply (see 1a7aa9a) -- e.g. url.query_params.time disagreeing on type across indices. Does not fix already-existing indices (mappings are immutable in place) -- see README.md Troubleshooting for the reindex/wipe-and-reingest guidance. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sudo runuser -u logstash -- env VAR=... bash cmd required every command to re-wrap the same env vars inline -- painful to write and execute. Matches the sudo runuser -l logstash pattern already adopted elsewhere in this doc (Access model, Day-2 operations): drop into a real login shell once, then run plain commands. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 91 out of 102 changed files in this pull request and generated no new comments.
Files not reviewed (3)
- terraform/iam/policies/.terraform.lock.hcl: Generated file
- terraform/logstash/.terraform.lock.hcl: Generated file
- terraform/s3/.terraform.lock.hcl: Generated file
Suppressed comments (7)
terraform/logstash/variables.tf:71
- ec2_security_group_name is documented as required when manage_ec2_instance=true, but there is no validation enforcing that. This currently fails later during the aws_security_group data lookup with a less clear error.
terraform/logstash/variables.tf:95 - mcp_ami_owner_id is documented as required when manage_ec2_instance=true, but there is no validation enforcing that. Without it, the AMI lookup will fail with an opaque provider error.
scripts/smoke-test.sh:16 - The smoke test hard-codes REGION to us-west-2. If this stack is deployed in another region (aws_region is a tfvar), the script will query the wrong SSM parameters and sign requests for the wrong region.
src/pds/web_analytics/egress_report.py:166 - build_email_html() interpolates bucket keys directly into HTML. Since these values ultimately originate from log data / reverse DNS, they should be HTML-escaped to avoid HTML injection in the emailed report.
src/pds/web_analytics/egress_report.py:133 - _sign_request() signs with the session's refreshable credentials object directly. It’s safer to freeze credentials before signing, and to fail with a clear error if no credentials are available (otherwise SigV4Auth will raise less actionable errors later).
terraform/logstash/variables.tf:65 - vpc_id is documented as required when manage_ec2_instance=true, but there is no validation enforcing that. When omitted, Terraform will fail later in data source resolution with a less clear error. Adding variable validation makes failures immediate and actionable.
This issue also appears in the following locations of the same file:
- line 67
- line 91
terraform/logstash/variables.tf:46
- ec2_role_name is used both as a role name (for IAM/S3 policy ARNs) and as an EC2 instance profile name (aws_launch_template.iam_instance_profile.name). Those are distinct IAM resources and don’t always share the same name, so this variable can be mis-set in environments where they differ. At minimum, the variable description should explicitly state that the role and instance profile names must match for this module to work.
0 is only correct for a single-data-node domain (dev). This template is shared across pds-dev-*, pds-test-*, pds-prod-* -- hardcoding 0 would leave a multi-node prod domain with zero replication. Dev's yellow cluster health from a single-node domain needs a per-venue fix, not a change to the shared default; use the one-time _settings PUT for existing dev indices instead (see chat/README Troubleshooting). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 91 out of 102 changed files in this pull request and generated 1 comment.
Files not reviewed (3)
- terraform/iam/policies/.terraform.lock.hcl: Generated file
- terraform/logstash/.terraform.lock.hcl: Generated file
- terraform/s3/.terraform.lock.hcl: Generated file
Suppressed comments (5)
src/pds/web_analytics/egress_report.py:285
run()logs and reports totals as GB, but it pullstotal_gbdirectly from the OpenSearch response where the aggregation is a sum of bytes. After converting inbuild_email_html(),run()should also convert before logging (and before the dry-run message).
src/pds/web_analytics/egress_report.py:162- OpenSearch aggregations in
_build_query()sumhttp.response.body.bytes(bytes), butbuild_email_html()formats those raw byte values as "GB" without converting. This will overstate egress by 1e9x and makesBYTES_PER_GBunused.
.detect-secrets-ignore:12 - Excluding the entire
terraform/tree from detect-secrets scanning creates a blind spot in a part of the repo where credentials/ARNs/endpoints often get accidentally committed. It’s safer to scan Terraform code and only exclude the known high-noise/secret-bearing artifacts (tfvars/tfstate/tfplan), which are already handled elsewhere.
# Documentation files contain placeholder/example credentials only
README\.md
CHANGELOG\.md
terraform/.*
src/pds/web_analytics/egress_report.py:275
smtp_config["server"].split(":")will raiseValueErrorif the server value contains additional colons (e.g., IPv6) or if a port is missing. Parsing as host/port should split from the right once.
tests/test_egress_report.py:26- The sample OpenSearch response uses values that are treated as GB, but the real aggregation values are bytes (sum of
http.response.body.bytes). With byte→GB conversion inEgressReporter, these fixtures should store bytes so the test reflects production behavior.
| "index": { | ||
| "number_of_shards": 3, | ||
| "number_of_replicas": 0, | ||
| "mapping": { | ||
| "total_fields": { |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 91 out of 102 changed files in this pull request and generated no new comments.
Files not reviewed (3)
- terraform/iam/policies/.terraform.lock.hcl: Generated file
- terraform/logstash/.terraform.lock.hcl: Generated file
- terraform/s3/.terraform.lock.hcl: Generated file
Suppressed comments (2)
src/pds/web_analytics/egress_report.py:80
- The aggregations in
_build_query()sumhttp.response.body.bytes(bytes), but the code treats the returned values as GB (e.g.,total_gbandb['gb']['value']are rendered as “GB” without conversion). This will overstate download volume by ~1e9 and makes theBYTES_PER_GBconstant unused.
Consider either (a) converting bytes→GB when rendering (divide by BYTES_PER_GB), or (b) changing the aggregation to use a script so OpenSearch returns GB values. The unit tests’ sample response would need to match whichever approach you choose.
scripts/logstash-bootstrap.sh:52
dnf install python3.13/gettext(and the pip install of boto3/requests) only runs when Logstash is not already installed. On an existing/reused instance where Logstash is already present, bootstrap will skip installing these dependencies, but later scripts (logstash-deploy.sh,smoke-test.sh, egress report cron) assumepython3.13,envsubst(gettext), andgitexist.
Install the OS/Python dependencies unconditionally (or at least gate them on the commands being missing), and keep the Logstash RPM install conditional.
index_patterns was [pds-dev-*, pds-test-*, pds-prod-*] -- none of which
match the actual index naming convention (${INDEX_PREFIX}-YYYY-MM,
INDEX_PREFIX=pds-weblogs everywhere this has actually been deployed, and
matching the pds-weblogs* Dashboards index-pattern). The custom ECS
template has therefore never applied to a single real index -- confirmed
by fresh indices still getting OpenSearch's default 5 shards (template
specifies 3) and dynamic-mapping type inference (text vs long depending on
whether the first http.request.body.bytes value seen looked numeric,
e.g. "-" for zero-byte responses) rather than the template's explicit
mappings. Every earlier mapping/type fix in this file has been inert until
now. Added pds-weblogs-* rather than replacing the existing patterns, in
case another venue genuinely uses a pds-dev-*/pds-test-*/pds-prod-* prefix.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Only pds-weblogs-* is ever actually used (INDEX_PREFIX everywhere this deploys); the other three never matched anything and just added confusion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 91 out of 102 changed files in this pull request and generated 1 comment.
Files not reviewed (3)
- terraform/iam/policies/.terraform.lock.hcl: Generated file
- terraform/logstash/.terraform.lock.hcl: Generated file
- terraform/s3/.terraform.lock.hcl: Generated file
Suppressed comments (3)
src/pds/web_analytics/egress_report.py:166
- The OpenSearch aggregations in
_build_query()sumhttp.response.body.bytes, butbuild_email_html()treats the returned values as already being in GB (and labels them as GB). This will over-report by ~1e9×. Convert the summed byte counts to GB using BYTES_PER_GB before rendering.
src/pds/web_analytics/egress_report.py:293 run()logstotal_gbas GB, but it’s still the raw byte sum from the OpenSearch aggregation. Convert to GB (same as the email rendering) so dry-run and sent-report logs aren’t misleading.
tests/test_egress_report.py:18- The sample OpenSearch response values are currently treated as GB, but the query sums a byte field (
http.response.body.bytes). If the production code converts bytes → GB, the fixture should store byte values so the tests still validate the formatting correctly.
| CRON_CMD="PYTHONPATH=$REPO_DIR/src AWS_REGION=$AWS_REGION python3.13 $REPO_DIR/scripts/egress_report.py --opensearch-endpoint $OPENSEARCH_ENDPOINT --index-pattern ${INDEX_PREFIX}-* --region $AWS_REGION --smtp-env-file $SMTP_ENV_FILE --smtp-config-ssm-path $SMTP_CONFIG_SSM_KEY_PATH --recipients $EGRESS_REPORT_RECIPIENTS --hours $EGRESS_REPORT_HOURS >> /var/log/egress-report.log 2>&1" | ||
| (crontab -l 2>/dev/null | grep -v '# egress-report'; echo "$EGRESS_REPORT_SCHEDULE $CRON_CMD # egress-report") | crontab - |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 91 out of 102 changed files in this pull request and generated no new comments.
Files not reviewed (3)
- terraform/iam/policies/.terraform.lock.hcl: Generated file
- terraform/logstash/.terraform.lock.hcl: Generated file
- terraform/s3/.terraform.lock.hcl: Generated file
Suppressed comments (4)
src/pds/web_analytics/egress_report.py:104
- _build_query() aggregates
http.response.body.bytesbut names the aggregationtotal_gband the docstring says it is converted to GB. As written, the values returned are bytes and will be labeled as GB in the report. Consider converting to GB in the aggregation itself (scripted sum) so the response matches the name/label and avoids needing conversion logic in multiple places.
src/pds/web_analytics/egress_report.py:166 - build_email_html() interpolates aggregation bucket keys directly into HTML without escaping. If a bucket key contains characters like
</&, it can break the email markup or enable HTML injection in the generated report. Escaping keys before insertion makes the output robust.
scripts/smoke-test.sh:18 - The smoke test hardcodes
REGION = 'us-west-2', but the Terraform modules and scripts support overriding AWS region via variables/env. If this stack is ever deployed or tested outside us-west-2, the smoke test will query the wrong region's SSM parameters and fail even when the infrastructure is healthy. ReadingAWS_REGIONfrom the environment (with a default) makes it consistent with the rest of the tooling.
terraform/logstash/main.tf:71 - This passes
index_prefix = var.resource_prefixinto userdata, which setsINDEX_PREFIXon the EC2. However the index template inconfig/opensearch/ecs-8.17-custom-template.jsonis scoped topds-weblogs-*, andlogstash-deploy.shalso defaultsINDEX_PREFIXtopds-weblogs. Ifresource_prefixis something likepds-dev, Logstash will write topds-dev-YYYY-MMand the template/patterns won’t apply as intended. Consider using a dedicated index prefix (e.g.pds-weblogs) here, or updating the template patterns to match the chosen prefix consistently.
Summary
Replaces the previous AOSS-based Terraform with a complete managed OpenSearch infrastructure for the PDS Web Analytics pipeline.
What this deploys (this repo):
Key design decisions:
iam:deploy,logstash:deploy) split into dedicated tasks so Power-User can deploy S3 independently.tffiles — all environment-specific values in gitignored.tfvarsDeployment order:
task iam:deploy VENUE=dev— IAM policy (admin:iam:CreatePolicy,iam:AttachRolePolicy)task s3:deploy VENUE=dev— S3 log bucket (Power-User)task logstash:deploy VENUE=dev— Logstash EC2 (admin:iam:PassRole)🤖 Generated with Claude Code
Test plan
/pds/observability/opensearch/opensearch_endpoint,/pds/observability/opensearch/opensearch_arntask iam:plan VENUE=devreviewed and appliedtask s3:plan VENUE=devreviewed and appliedtask logstash:plan VENUE=devreviewed — confirms EC2 replace (-/+) from existing Docker-based instancetask logstash:deploy VENUE=devapplied — new EC2 created with RPM Logstash + systemdpds-dev-observability(new domain)pds-dev-web-analyticsOpenSearch domain after new EC2 confirmed healthyTesting Completed
I can confirm logstash is able to run in Dev and I have dashboards working with some test logs.

