Skip to content

⭐ Add AWS Route 53 resources#6704

Merged
tas50 merged 7 commits intomainfrom
tas50/route53
Mar 1, 2026
Merged

⭐ Add AWS Route 53 resources#6704
tas50 merged 7 commits intomainfrom
tas50/route53

Conversation

@tas50
Copy link
Copy Markdown
Member

@tas50 tas50 commented Feb 27, 2026

Summary

Add Route 53 support with new resource types:

  • aws.route53: Top-level resource with access to hosted zones, health checks,
    and query logging configs
  • aws.route53.hostedZone: DNS zones with records, VPCs, tags, DNSSEC status,
    nameservers, query logging, and key signing keys
  • aws.route53.record: DNS record sets with support for alias records, weighted/
    latency/failover/geolocation routing policies, and health check associations
  • aws.route53.healthCheck: HTTP/HTTPS/TCP/calculated/CloudWatch health checks
    with full configuration, tags, and status
  • aws.route53.queryLoggingConfig: Query logging configs with CloudWatch targets
  • aws.route53.keySigningKey: DNSSEC key signing keys with KMS key references

Includes Route 53 client caching in the AWS connection layer, cross-resource
references (records -> health checks, KSKs -> KMS keys, instances -> zones),
and proper handling of private hosted zones (DNSSEC not supported).

> aws.route53.hostedZones.first{*}
aws.route53.hostedZones.first: {
  tags: {}
  isPrivate: true
  id: "/hostedzone/1234"
  resourceRecordSetCount: 3
  vpcs: [
    0: {
      vpcId: "vpc-1234"
      vpcRegion: "us-west-2"
    }
  ]
  name: "foo.com."
  nameServers: []
  type: "PRIVATE"
  arn: "arn:aws:route53:::hostedzone/1234"
  queryLoggingConfig: null
  dnssecStatus: {
    serveSignature: "NOT_SIGNING"
    statusMessage: "DNSSEC is not supported for private hosted zones"
  }
  comment: "internal stuff"
  config: {
    comment: "internal stuff"
    privateZone: true
  }
  records: [
    0: aws.route53.record name="foo.com." type="NS"
    1: aws.route53.record name="foo.com." type="SOA"
    2: aws.route53.record name="tim.foo.com." type="A"
  ]
  keySigningKeys: []
}
aws.route53.healthChecks.first: {
  inverted: false
  callerReference: "12345"
  cloudWatchAlarmConfiguration: {}
  failureThreshold: 3
  port: 80
  id: "12345"
  measureLatency: false
  ipAddress: "63.192.209.236"
  childHealthChecks: []
  fullyQualifiedDomainName: ""
  status: "Failure: Connection timed out. The endpoint or the internet connection is down, or requests are being blocked by your firewall. See https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/dns-failover-router-firewall-rules.html"
  requestInterval: 30
  resourcePath: "/healthcheck"
  type: "HTTP_STR_MATCH"
  enableSNI: false
  searchString: "foo"
  disabled: false
  regions: [
    0: "ap-northeast-1"
    1: "ap-southeast-1"
    2: "ap-southeast-2"
    3: "eu-west-1"
    4: "sa-east-1"
    5: "us-east-1"
    6: "us-west-1"
    7: "us-west-2"
  ]
  tags: {
    Name: "test"
  }
  healthThreshold: 0
  protocol: "HTTP"
  arn: "arn:aws:route53:::healthcheck/12345"
}

@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 27, 2026

Test Results

5 167 tests  +31   5 163 ✅ +31   2m 15s ⏱️ +16s
  409 suites ± 0       4 💤 ± 0 
   31 files   ± 0       0 ❌ ± 0 

Results for commit cbae34d. ± Comparison against base commit 45e1726.

♻️ This comment has been updated with latest results.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Route 53 implementation is mostly solid, but health check status field is incorrectly populated with version number instead of actual status

@tas50 tas50 force-pushed the tas50/route53 branch 11 times, most recently from 332fe0d to 44f743d Compare February 28, 2026 13:32
@mondoo-code-review mondoo-code-review bot dismissed their stale review February 28, 2026 13:35

Superseded by new review

Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are pre-existing issues — the mock files (mock_coordinator.go, mock_schema.go, mock_plugin_interface.go, mock_volumemounter.go) are absent from the repo because go generate / make mql/generate was never run after the //go:generate mockgen ... directives were added. None of the files involved (providers/coordinator.go, providers/runtime_test.go, providers/coordinator_test.go, providers/os/connection/snapshot/volumemounter.go) were touched by the Route 53 PR.

To fix them, run the go:generate directives:

# From repo root
go generate ./providers/...
go generate ./providers/os/connection/snapshot/...

Or via the full pipeline:

make mql/generate

That will create:

  • providers/mock_coordinator.go
  • providers/mock_plugin_interface.go
  • providers/mock_schema.go
  • providers/os/connection/snapshot/mock_volumemounter.go

These errors are not related to the Route 53 PR and were present before it landed.

Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid Route 53 implementation with good pagination and caching, but has nil pointer dereference risks on API responses and a few unchecked pointer dereferences inside field mappings.

Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid implementation with one nil-dereference risk, redundant API calls, and a PR description that claims resources (delegationSet, trafficPolicy, trafficPolicyInstance) that don't exist in the actual diff.

Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid Route 53 implementation, but two methods redundantly call GetDNSSEC and one method does a full health-check list scan when a direct API call is available.

Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid implementation with good pagination and error handling, but has three notable performance issues.

@mondoo-code-review
Copy link
Copy Markdown

Unable to complete the code review.

Reason: An internal error occurred during the review.

Error details
claude failed with exit code: signal: interrupt

You can try /review again or reduce the PR size.

@mondoo-code-review mondoo-code-review bot dismissed their stale review February 28, 2026 14:53

Dismissed: review could not be completed

@tas50
Copy link
Copy Markdown
Member Author

tas50 commented Feb 28, 2026

/review

Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well-structured Route 53 resource implementation with good patterns; a few correctness and robustness issues to address.

@tas50 tas50 added the manually-tested PR content has been manually tested against real assets label Feb 28, 2026
Add comprehensive Route 53 support with 8 new resource types:
- aws.route53: Top-level resource with access to hosted zones, health checks,
  delegation sets, traffic policies, query logging configs, and DNSSEC keys
- aws.route53.hostedZone: DNS zones with records, VPCs, tags, DNSSEC status,
  nameservers, query logging, and traffic policy instances
- aws.route53.record: DNS record sets with support for alias records, weighted/
  latency/failover/geolocation routing policies, and health check associations
- aws.route53.healthCheck: HTTP/HTTPS/TCP/calculated/CloudWatch health checks
  with full configuration, tags, and status
- aws.route53.delegationSet: Reusable delegation sets with nameservers
- aws.route53.trafficPolicy: Traffic policies with document and instances
- aws.route53.trafficPolicyInstance: Traffic policy instances linked to zones
- aws.route53.queryLoggingConfig: Query logging configs with CloudWatch targets
- aws.route53.keySigningKey: DNSSEC key signing keys with KMS key references

Includes Route 53 client caching in the AWS connection layer, cross-resource
references (records -> health checks, KSKs -> KMS keys, instances -> zones),
and proper handling of private hosted zones (DNSSEC not supported).

Signed-off-by: Tim Smith <tsmith84@gmail.com>
tas50 and others added 6 commits March 1, 2026 06:41
- Add nil checks on GetHostedZone responses in vpcs() and nameServers()
  to prevent potential nil pointer dereference on partial failures
- Add nil checks on GetDNSSEC responses in dnssecStatus() and
  keySigningKeys() for the same reason
- Remove explicit State setting in queryLoggingConfig() for consistency
  with vpcs(), nameServers(), dnssecStatus(), and keySigningKeys()
  which all simply return nil, nil on access-denied or empty results

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ument status()

- Cache GetHostedZone response in mqlAwsRoute53HostedZoneInternal so
  vpcs() and nameServers() share a single API call instead of making
  duplicate requests for the same hosted zone
- Add nil guard for hc.HealthCheckConfig to prevent panic if the SDK
  ever returns a health check with nil config
- Document that status() intentionally returns a single representative
  status from the first health checker region observation

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Test newMqlAwsRoute53Record with 8 cases:
- basic A record, alias record, weighted routing, geolocation,
  geoproximity with bias, CIDR routing, nil optional defaults, ID format

Test newMqlAwsRoute53HealthCheck with 5 cases:
- HTTP with all fields, CALCULATED with children, CloudWatch alarm
  configuration, nil HealthCheckConfig error, nil optional defaults

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…h tag fetching

- Cache GetDNSSEC API call so dnssecStatus() and keySigningKeys() share a single
  request per hosted zone, matching the existing getHostedZone() caching pattern.
- Use GetHealthCheck (singular) API for O(1) lookup in record.healthCheck()
  instead of fetching the entire paginated health check list.
- Batch tag fetching with ListTagsForResources (plural, up to 10 IDs per call)
  for both hostedZones() and healthChecks(), reducing API calls ~10x.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Tim Smith <tsmith84@gmail.com>
AWS allows at most one query logging config per hosted zone, so
pagination is not needed in the per-zone lookup.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mondoo-code-review mondoo-code-review bot dismissed their stale review March 1, 2026 14:42

Superseded by new review

Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous review feedback well addressed; new tests are clean and comprehensive.

@tas50 tas50 merged commit 48b3140 into main Mar 1, 2026
19 of 20 checks passed
@tas50 tas50 deleted the tas50/route53 branch March 1, 2026 15:09
@github-actions github-actions bot locked and limited conversation to collaborators Mar 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

manually-tested PR content has been manually tested against real assets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant