fix(aws,k8s): guard original tag auditors against nil clients; assert k8s provider interface#30
Merged
Conversation
…sert the k8s provider interface
Two consistency fixes from the core-repos audit.
─── tags.go: nil-client guards on the original four auditors ───
The first four tag auditors (EC2, S3, RDS, Lambda) dereferenced their AWS
client immediately, while the newer five (ECS, EKS, DynamoDB, SNS, SQS) added
in 7c open with an `if p.<client> == nil { return nil, nil }` guard. A Provider
built with only some clients wired — exactly how the per-service tests construct
it — would panic in the first four when `AuditTags` dispatched to them with a
required-tags list. Added the same guard to all four so partial construction is
safe, plus a `TestAuditTags_NilClientsNoPanic` regression test that drives the
dispatch path with no clients wired (the existing no-required test returns before
dispatch, so it never exercised this).
─── k8s: compile-time interface assertion ───
`Provider.Name()` and `Provider.Detect()` looked like dead code (callers use the
concrete `*k8s.Provider`, so nothing references them), but they're required by
`cloud.K8sRBACProvider` — the interface the package is documented to implement.
Deleting them would silently break that contract. Added
`var _ cloud.K8sRBACProvider = (*Provider)(nil)` in rbac.go, mirroring the AWS
provider's `var _ cloud.QuotaProvider = (*Provider)(nil)` — it documents the
intent and makes the compiler enforce the full method set (Name, Detect,
ContextName, ScanRBAC) against future drift.
`go build`, `go vet`, `go test ./...`, golangci-lint, and the per-package
coverage floors all pass.
Closes #29
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
1 participant
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.
What
Two consistency fixes from audit follow-up #29.
tags.go— nil-client guards on the original four auditorsThe first four tag auditors (EC2, S3, RDS, Lambda) dereferenced their AWS client immediately, while the newer five (ECS/EKS/DynamoDB/SNS/SQS) open with
if p.<client> == nil { return nil, nil }. AProviderwith only some clients wired — how the per-service tests construct it — would panic in the first four onceAuditTagsdispatched with a required-tags list. Added the same guard to all four, plusTestAuditTags_NilClientsNoPanicdriving the dispatch path with no clients wired (the existing no-required test returns before dispatch, so it never covered this).k8s— compile-time interface assertion (not deletion)Provider.Name()/Detect()look like dead code (callers use the concrete*k8s.Provider), but they're required bycloud.K8sRBACProvider— the interface the package is documented to implement. Deleting them would silently break that contract. Addedvar _ cloud.K8sRBACProvider = (*Provider)(nil)inrbac.go, mirroring AWS'svar _ cloud.QuotaProvider = (*Provider)(nil)— documents intent and makes the compiler enforce the full method set against drift.Verification
go build,go vet,go test ./...,golangci-lint run(0 issues), and the per-package coverage floors all pass locally.Closes #29