diff --git a/.github/workflows/chart-provenance.yml b/.github/workflows/chart-provenance.yml index e196fec..1ef38db 100644 --- a/.github/workflows/chart-provenance.yml +++ b/.github/workflows/chart-provenance.yml @@ -82,3 +82,30 @@ jobs: # moved upstream underneath it. - name: Re-measure every pinned directory source run: ./scripts/check-directory-manifest-size.py --live + + scheme-inputs: + name: the controller still decides schemes the way the policy reads them + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + + - name: Install Helm + uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 # v5 + + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version-file: .python-version + - name: Install Python dependencies + run: pip install --require-hashes -r requirements.txt + + # The blocking half compares the record against the chart pin, so it fails + # the moment somebody moves the controller. What it cannot see is the + # controller growing a new way to decide a scheme between bumps, or the + # chart changing which flags it renders from the same values. Both need the + # source and a render, and both are read here. + - name: Re-derive the scheme inputs from the pinned controller + run: ./scripts/check-lb-scheme-inputs.py --live diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1cf52b2..d703878 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -480,6 +480,23 @@ jobs: - name: Run policy unit tests run: ./scripts/kyverno-test.sh policies/kyverno/tests + # The subnet-injection policy decides private-or-public from a scheme it + # believes a load balancer will have, and that belief is only as wide as the + # set of inputs it reads. The AWS Load Balancer Controller decides the same + # thing from four annotations, an IngressClassParams field, an Ingress + # group, and a load balancer that already exists in AWS. This derives that + # set from the controller source at the pinned chart version and fails when + # one of them is neither read by the policy nor recorded with the reason it + # is not — so a spelling nobody has heard of yet lands red rather than + # quietly taking the default. Offline: the derivation is keyed on the chart + # pin, so moving the pin fails here, and upstream/pins re-derives on a + # schedule. + - name: Every input the controller decides a scheme on is one the policy reads + run: | + pip install --require-hashes -r requirements.txt + ./scripts/check-lb-scheme-inputs.py + ./scripts/check-lb-scheme-inputs.py --self-test + # The unit tests above load BASE policies and pin verify-images' match and # exclude scoping only. This structural gate guards the signing-IDENTITY # contract they are blind to — required signature, GitHub OIDC issuer, diff --git a/CLAUDE.md b/CLAUDE.md index 6dffc19..06030b1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -101,6 +101,7 @@ task validate:policy-admission # Prove no addon is denied by the Enforce-tie task validate:externalsecret-keys # Each ExternalSecret names its remote secret once, and the delivering appset patches it task validate:secret-store-refs # Every secret-store reference names the one store this catalog declares, and the published contract states it task validate:directory-manifest-size # Every directory-source Application fits the repo-server's combined-manifest ceiling +task validate:lb-scheme-inputs # Every input the AWS LB Controller decides a scheme on is one the injection policy reads task validate:dashboards # grafana.com dashboard ids exist and are AMG-saveable task validate:athena-panel-columns # Every column a CUR panel names is one the export delivers task validate:fork-safety # No hardcoded catalog repoURL in applied ApplicationSets (report-only locally) @@ -114,7 +115,8 @@ task validate:image-vulnerabilities # Every fixed CRITICAL in a rendered image i `task validate` runs the structural gates (lint, kustomize build, helm-render, ApplicationSet schema, sync-wave ordering, appset render, policy-admission, -secret-store references, directory-source sizes, dashboards, fork-safety). CI runs those plus several gates that have **no local +secret-store references, directory-source sizes, load-balancer scheme inputs, +dashboards, fork-safety). CI runs those plus several gates that have **no local `task` target**, and one that has a target but is deliberately outside the aggregate, so a clean `task validate` is necessary but not sufficient: @@ -179,6 +181,23 @@ aggregate, so a clean `task validate` is necessary but not sufficient: delivered panel measures is a finding, because a figure compared to nothing is how the last wrong one survived. The figure has no independent existence: there is no constant to correct and none in the summary that anything trusts +- **Load-balancer scheme inputs** — `scripts/check-lb-scheme-inputs.py`, in the + `kyverno` job. `inject-adopt-lb-subnets` injects private or public subnet ids + according to the scheme it believes a load balancer will have, and reading one + annotation per object kind made that belief a pattern: a second spelling of the + same thing, `aws-load-balancer-internal`, is still honoured and still ahead of + the controller's default, so a Service setting only that one was internet-facing + to the controller and internal to the policy. Adding the second annotation fixes + the instance and not the next one, so the population is derived instead: the + gate reads `buildLoadBalancerScheme` (Service and Ingress) and + `IsServiceSupported` in the controller source at the version the chart pin + installs, follows the functions they call, and requires every symbol those + bodies consult to be recorded in `scripts/lb-scheme-inputs.json` as READ with a + string the policy must contain, UNREAD with the reason it cannot be consulted, + or PLUMBING with the reason it decides nothing. A symbol that is none of those + fails `--sync`. The record is keyed on the chart pin, so moving the controller + fails the blocking half; `--live` re-derives from source and re-renders the + chart on the schedule, because a controller can grow an input between bumps - **Directory-source manifest sizes** — `scripts/check-directory-manifest-size.py`, in the `appsets` job beside the offline chart-provenance half. The repo-server refuses to generate a directory-type Application whose combined manifest files @@ -232,8 +251,10 @@ documents: `task validate` runs it report-only, CI runs it `--blocking`. `scripts/check-directory-manifest-size.py --live`, which re-measures each pinned directory source: the blocking half already fails when a pin moves away from its measurement, and what only a clone can answer is whether a tag nobody - moved here was moved upstream underneath it. Both need the network, which is - why neither is on the merge path + moved here was moved upstream underneath it. A third runs + `scripts/check-lb-scheme-inputs.py --live`, which re-derives the inputs the AWS + Load Balancer Controller decides a load balancer's scheme from. All three need + the network, which is why none is on the merge path - Manual diff rendering available via `.github/workflows/diff.yml` ## Claude Code Tooling diff --git a/Taskfile.yaml b/Taskfile.yaml index 4476021..af28028 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -206,13 +206,19 @@ tasks: - ./scripts/check-directory-manifest-size.py - ./scripts/check-directory-manifest-size.py --self-test + validate:lb-scheme-inputs: + desc: "Load-balancer scheme gate — every input the controller decides a scheme on is one the injection policy reads" + cmds: + - ./scripts/check-lb-scheme-inputs.py + - ./scripts/check-lb-scheme-inputs.py --self-test + validate:athena-panel-columns: desc: "Athena panel gate — every column a CUR panel names is one the export delivers" cmds: - ./scripts/check-athena-panel-columns.py validate: - desc: Run all validations (lint, build, helm-render, appset schema, sync waves, label values, appset render, policy admission, secret refs, directory sizes, athena panels, dashboard/fork-safety) + desc: Run every gate this repo can run offline — lint, build, helm-render, schema, sync waves, label values, appset render, policy admission, and the reference/derivation gates deps: - lint:yaml - lint:python @@ -226,6 +232,7 @@ tasks: - validate:externalsecret-keys - validate:secret-store-refs - validate:directory-manifest-size + - validate:lb-scheme-inputs - validate:athena-panel-columns - validate:dashboards - validate:fork-safety diff --git a/policies/kyverno/networking/base/inject-adopt-lb-subnets.yaml b/policies/kyverno/networking/base/inject-adopt-lb-subnets.yaml index 3e1e591..9ad8a3b 100644 --- a/policies/kyverno/networking/base/inject-adopt-lb-subnets.yaml +++ b/policies/kyverno/networking/base/inject-adopt-lb-subnets.yaml @@ -15,13 +15,14 @@ metadata: auto-discovery finds nothing. This policy supplies the subnets explicitly. It reads the cluster-local network-config ConfigMap (written by cluster-bootstrap in every mode) and, only when that record says the cluster - is in adopt mode, injects the subnet-id annotation the controller reads. It - is scheme-aware: an internal load balancer (the controller's default when no - scheme is set) lands on the private subnets, an internet-facing one on the - public subnets. The subnet annotation is added only if absent, so a tenant - that pins its own subnets is never overridden. A create-mode cluster owns its - VPC and its own ELB role tags, so the ConfigMap reports create, every rule's - precondition fails, and nothing is mutated. + is in adopt mode, injects the subnet-id annotation the controller reads. An + internal load balancer lands on the private subnets, an internet-facing one + on the public subnets, and the scheme is resolved the way the controller + resolves it rather than from a single annotation. The subnet annotation is + added only if absent, so a tenant that pins its own subnets is never + overridden. A create-mode cluster owns its VPC and its own ELB role tags, so + the ConfigMap reports create, every rule's precondition fails, and nothing + is mutated. spec: admission: true emitWarning: false @@ -41,7 +42,7 @@ spec: webhookConfiguration: failurePolicy: Ignore rules: - # ── Ingress, internal scheme (the default) → private subnets ────────────── + # ── Ingress, internal scheme → private subnets ──────────────────────────── - name: ingress-internal-subnets skipBackgroundRequests: true match: @@ -54,17 +55,36 @@ spec: configMap: name: network-config namespace: kube-system - # The object's own scheme, defaulted to internal to match the AWS Load - # Balancer Controller's default when the annotation is absent. - - name: scheme + # An Ingress that names a group shares one load balancer, and its scheme + # -- one scheme for the whole group -- is decided across every member. No + # single object carries the answer, and a member that disagrees with the + # group is rejected by the controller rather than given its own subnets. + # Injecting from this object alone would be a guess about the others. + - name: groupname + variable: + jmesPath: 'request.object.metadata.annotations."alb.ingress.kubernetes.io/group.name"' + default: "" + - name: schemeannotation variable: jmesPath: 'request.object.metadata.annotations."alb.ingress.kubernetes.io/scheme"' - default: internal + default: "" + # The controller's order for a single-member group: an IngressClassParams + # scheme, else this annotation, else its --default-load-balancer-scheme. + # IngressClassParams is not read here and that is a decision, not an + # oversight -- scripts/check-lb-scheme-inputs.py records which inputs are + # read and which are knowingly not, and fails when the controller grows + # one that is neither. + - name: scheme + variable: + value: "{{ schemeannotation || 'internal' }}" preconditions: all: - key: "{{ networkconfig.data.network_mode }}" operator: Equals value: adopt + - key: "{{ groupname }}" + operator: Equals + value: "" - key: "{{ scheme }}" operator: Equals value: internal @@ -90,15 +110,25 @@ spec: configMap: name: network-config namespace: kube-system - - name: scheme + - name: groupname + variable: + jmesPath: 'request.object.metadata.annotations."alb.ingress.kubernetes.io/group.name"' + default: "" + - name: schemeannotation variable: jmesPath: 'request.object.metadata.annotations."alb.ingress.kubernetes.io/scheme"' - default: internal + default: "" + - name: scheme + variable: + value: "{{ schemeannotation || 'internal' }}" preconditions: all: - key: "{{ networkconfig.data.network_mode }}" operator: Equals value: adopt + - key: "{{ groupname }}" + operator: Equals + value: "" - key: "{{ scheme }}" operator: Equals value: internet-facing @@ -110,7 +140,7 @@ spec: metadata: annotations: +(alb.ingress.kubernetes.io/subnets): "{{ networkconfig.data.public_subnet_ids }}" - # ── Service type=LoadBalancer, internal scheme (the default) → private ──── + # ── Service the controller provisions, internal scheme → private subnets ── - name: service-internal-subnets skipBackgroundRequests: true match: @@ -123,15 +153,61 @@ spec: configMap: name: network-config namespace: kube-system - - name: scheme + # WHICH SERVICES THE CONTROLLER PROVISIONS. A type=LoadBalancer Service is + # not automatically the AWS Load Balancer Controller's. It claims one whose + # spec.loadBalancerClass is its own class, or that carries + # aws-load-balancer-type: nlb-ip, or aws-load-balancer-type: external with + # an nlb-target-type. Every other type=LoadBalancer Service goes to the + # in-tree cloud provider, which builds a Classic Load Balancer and reads + # neither the scheme annotation nor the subnets annotation this policy + # writes. The chart sets enableServiceMutatorWebhook: false, so nothing + # stamps loadBalancerClass on a Service that does not ask for it, and the + # plain shape stays the in-tree provider's. + - name: lbclass + variable: + jmesPath: 'request.object.spec.loadBalancerClass' + default: "" + - name: lbtype + variable: + jmesPath: 'request.object.metadata.annotations."service.beta.kubernetes.io/aws-load-balancer-type"' + default: "" + - name: nlbtargettype + variable: + jmesPath: 'request.object.metadata.annotations."service.beta.kubernetes.io/aws-load-balancer-nlb-target-type"' + default: "" + - name: controllermanaged + variable: + value: "{{ lbclass == 'service.k8s.aws/nlb' || lbtype == 'nlb-ip' || (lbtype == 'external' && (nlbtargettype == 'ip' || nlbtargettype == 'instance')) }}" + - name: schemeannotation variable: jmesPath: 'request.object.metadata.annotations."service.beta.kubernetes.io/aws-load-balancer-scheme"' - default: internal + default: "" + # The legacy spelling, still honoured and still ahead of the default. + # aws-load-balancer-internal: "false" means internet-facing; a Service + # setting it without the newer scheme annotation is internet-facing to the + # controller and was internal to this policy, which is the private-subnet + # CSV on a load balancer the controller wants on public ones. + - name: internalannotation + variable: + jmesPath: 'request.object.metadata.annotations."service.beta.kubernetes.io/aws-load-balancer-internal"' + default: "" + # The controller's order: the scheme annotation, else the legacy internal + # flag, else the scheme of a load balancer that already exists, else its + # --default-load-balancer-scheme. The existing load balancer is not + # readable from an admission request at all; the default is asserted + # against the rendered chart by scripts/check-lb-scheme-inputs.py rather + # than assumed here. + - name: scheme + variable: + value: "{{ schemeannotation || (internalannotation == 'false' && 'internet-facing' || 'internal') }}" preconditions: all: - key: "{{ request.object.spec.type }}" operator: Equals value: LoadBalancer + - key: "{{ controllermanaged }}" + operator: Equals + value: true - key: "{{ networkconfig.data.network_mode }}" operator: Equals value: adopt @@ -146,7 +222,7 @@ spec: metadata: annotations: +(service.beta.kubernetes.io/aws-load-balancer-subnets): "{{ networkconfig.data.private_subnet_ids }}" - # ── Service type=LoadBalancer, internet-facing scheme → public subnets ──── + # ── Service the controller provisions, internet-facing → public subnets ─── - name: service-internet-facing-subnets skipBackgroundRequests: true match: @@ -159,15 +235,40 @@ spec: configMap: name: network-config namespace: kube-system - - name: scheme + - name: lbclass + variable: + jmesPath: 'request.object.spec.loadBalancerClass' + default: "" + - name: lbtype + variable: + jmesPath: 'request.object.metadata.annotations."service.beta.kubernetes.io/aws-load-balancer-type"' + default: "" + - name: nlbtargettype + variable: + jmesPath: 'request.object.metadata.annotations."service.beta.kubernetes.io/aws-load-balancer-nlb-target-type"' + default: "" + - name: controllermanaged + variable: + value: "{{ lbclass == 'service.k8s.aws/nlb' || lbtype == 'nlb-ip' || (lbtype == 'external' && (nlbtargettype == 'ip' || nlbtargettype == 'instance')) }}" + - name: schemeannotation variable: jmesPath: 'request.object.metadata.annotations."service.beta.kubernetes.io/aws-load-balancer-scheme"' - default: internal + default: "" + - name: internalannotation + variable: + jmesPath: 'request.object.metadata.annotations."service.beta.kubernetes.io/aws-load-balancer-internal"' + default: "" + - name: scheme + variable: + value: "{{ schemeannotation || (internalannotation == 'false' && 'internet-facing' || 'internal') }}" preconditions: all: - key: "{{ request.object.spec.type }}" operator: Equals value: LoadBalancer + - key: "{{ controllermanaged }}" + operator: Equals + value: true - key: "{{ networkconfig.data.network_mode }}" operator: Equals value: adopt diff --git a/policies/kyverno/tests/networking-adopt/kyverno-test.yaml b/policies/kyverno/tests/networking-adopt/kyverno-test.yaml index 21bffa5..10ba282 100644 --- a/policies/kyverno/tests/networking-adopt/kyverno-test.yaml +++ b/policies/kyverno/tests/networking-adopt/kyverno-test.yaml @@ -2,12 +2,25 @@ # tree) by `kyverno test policies/kyverno/tests`. The kube-system/network-config # ConfigMap state is supplied via values.yaml. # -# (a) plain Ingress → private-subnet CSV (internal is the default) -# (b) internet-facing Ingress→ public-subnet CSV -# (c) plain LB Service → private-subnet CSV -# (d) internet-facing Service→ public-subnet CSV -# (f) Ingress with an explicit subnets annotation → untouched (skip) -# (+) ClusterIP Service → untouched, proving the type precondition (skip) +# The matrix is the AWS Load Balancer Controller's own decision procedure: first +# whether the object is one it provisions a load balancer for, then which scheme +# it will compute for it. +# +# Ingress +# (a) plain → private-subnet CSV (the controller's default) +# (b) internet-facing → public-subnet CSV +# (c) already pins subnets → untouched +# (d) member of a group → untouched, the scheme belongs to the group +# Service the controller claims +# (e) by loadBalancerClass → private +# (f) by type=external + target → private +# (g) by type=nlb-ip, scheme set → public +# (h) legacy -internal: "false" → PUBLIC, with no scheme annotation present +# (i) legacy -internal: "true" → private +# (j) both spellings disagreeing → private, the scheme annotation wins +# Service the controller does not claim +# (k) plain type=LoadBalancer → untouched, the in-tree provider's Classic LB +# (l) ClusterIP → untouched apiVersion: cli.kyverno.io/v1alpha1 kind: Test metadata: @@ -30,11 +43,27 @@ results: resources: [ing-internet] patchedResources: patched/ing-internet.yaml result: pass + - policy: inject-adopt-lb-subnets + rule: ingress-internal-subnets + kind: Ingress + resources: [ing-explicit] + result: skip + - policy: inject-adopt-lb-subnets + rule: ingress-internal-subnets + kind: Ingress + resources: [ing-grouped] + result: skip - policy: inject-adopt-lb-subnets rule: service-internal-subnets kind: Service - resources: [svc-plain] - patchedResources: patched/svc-plain.yaml + resources: [svc-nlb-class] + patchedResources: patched/svc-nlb-class.yaml + result: pass + - policy: inject-adopt-lb-subnets + rule: service-internal-subnets + kind: Service + resources: [svc-external-instance] + patchedResources: patched/svc-external-instance.yaml result: pass - policy: inject-adopt-lb-subnets rule: service-internet-facing-subnets @@ -43,9 +72,27 @@ results: patchedResources: patched/svc-internet.yaml result: pass - policy: inject-adopt-lb-subnets - rule: ingress-internal-subnets - kind: Ingress - resources: [ing-explicit] + rule: service-internet-facing-subnets + kind: Service + resources: [svc-legacy-internal-false] + patchedResources: patched/svc-legacy-internal-false.yaml + result: pass + - policy: inject-adopt-lb-subnets + rule: service-internal-subnets + kind: Service + resources: [svc-legacy-internal-true] + patchedResources: patched/svc-legacy-internal-true.yaml + result: pass + - policy: inject-adopt-lb-subnets + rule: service-internal-subnets + kind: Service + resources: [svc-scheme-beats-legacy] + patchedResources: patched/svc-scheme-beats-legacy.yaml + result: pass + - policy: inject-adopt-lb-subnets + rule: service-internal-subnets + kind: Service + resources: [svc-plain] result: skip - policy: inject-adopt-lb-subnets rule: service-internal-subnets diff --git a/policies/kyverno/tests/networking-adopt/patched/svc-external-instance.yaml b/policies/kyverno/tests/networking-adopt/patched/svc-external-instance.yaml new file mode 100644 index 0000000..7b7e2b0 --- /dev/null +++ b/policies/kyverno/tests/networking-adopt/patched/svc-external-instance.yaml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: Service +metadata: + name: svc-external-instance + namespace: workloads + annotations: + service.beta.kubernetes.io/aws-load-balancer-type: external + service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: instance + service.beta.kubernetes.io/aws-load-balancer-subnets: subnet-priv-a,subnet-priv-b +spec: + type: LoadBalancer + ports: + - port: 80 diff --git a/policies/kyverno/tests/networking-adopt/patched/svc-internet.yaml b/policies/kyverno/tests/networking-adopt/patched/svc-internet.yaml index 5ee2df2..ef8e188 100644 --- a/policies/kyverno/tests/networking-adopt/patched/svc-internet.yaml +++ b/policies/kyverno/tests/networking-adopt/patched/svc-internet.yaml @@ -4,6 +4,7 @@ metadata: name: svc-internet namespace: workloads annotations: + service.beta.kubernetes.io/aws-load-balancer-type: nlb-ip service.beta.kubernetes.io/aws-load-balancer-scheme: internet-facing service.beta.kubernetes.io/aws-load-balancer-subnets: subnet-pub-a,subnet-pub-b spec: diff --git a/policies/kyverno/tests/networking-adopt/patched/svc-legacy-internal-false.yaml b/policies/kyverno/tests/networking-adopt/patched/svc-legacy-internal-false.yaml new file mode 100644 index 0000000..1aaf048 --- /dev/null +++ b/policies/kyverno/tests/networking-adopt/patched/svc-legacy-internal-false.yaml @@ -0,0 +1,14 @@ +apiVersion: v1 +kind: Service +metadata: + name: svc-legacy-internal-false + namespace: workloads + annotations: + service.beta.kubernetes.io/aws-load-balancer-type: external + service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: ip + service.beta.kubernetes.io/aws-load-balancer-internal: "false" + service.beta.kubernetes.io/aws-load-balancer-subnets: subnet-pub-a,subnet-pub-b +spec: + type: LoadBalancer + ports: + - port: 80 diff --git a/policies/kyverno/tests/networking-adopt/patched/svc-legacy-internal-true.yaml b/policies/kyverno/tests/networking-adopt/patched/svc-legacy-internal-true.yaml new file mode 100644 index 0000000..818fab2 --- /dev/null +++ b/policies/kyverno/tests/networking-adopt/patched/svc-legacy-internal-true.yaml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: Service +metadata: + name: svc-legacy-internal-true + namespace: workloads + annotations: + service.beta.kubernetes.io/aws-load-balancer-internal: "true" + service.beta.kubernetes.io/aws-load-balancer-subnets: subnet-priv-a,subnet-priv-b +spec: + type: LoadBalancer + loadBalancerClass: service.k8s.aws/nlb + ports: + - port: 80 diff --git a/policies/kyverno/tests/networking-adopt/patched/svc-plain.yaml b/policies/kyverno/tests/networking-adopt/patched/svc-nlb-class.yaml similarity index 77% rename from policies/kyverno/tests/networking-adopt/patched/svc-plain.yaml rename to policies/kyverno/tests/networking-adopt/patched/svc-nlb-class.yaml index 0ffa322..0352e85 100644 --- a/policies/kyverno/tests/networking-adopt/patched/svc-plain.yaml +++ b/policies/kyverno/tests/networking-adopt/patched/svc-nlb-class.yaml @@ -1,11 +1,12 @@ apiVersion: v1 kind: Service metadata: - name: svc-plain + name: svc-nlb-class namespace: workloads annotations: service.beta.kubernetes.io/aws-load-balancer-subnets: subnet-priv-a,subnet-priv-b spec: type: LoadBalancer + loadBalancerClass: service.k8s.aws/nlb ports: - port: 80 diff --git a/policies/kyverno/tests/networking-adopt/patched/svc-scheme-beats-legacy.yaml b/policies/kyverno/tests/networking-adopt/patched/svc-scheme-beats-legacy.yaml new file mode 100644 index 0000000..152079d --- /dev/null +++ b/policies/kyverno/tests/networking-adopt/patched/svc-scheme-beats-legacy.yaml @@ -0,0 +1,14 @@ +apiVersion: v1 +kind: Service +metadata: + name: svc-scheme-beats-legacy + namespace: workloads + annotations: + service.beta.kubernetes.io/aws-load-balancer-type: nlb-ip + service.beta.kubernetes.io/aws-load-balancer-scheme: internal + service.beta.kubernetes.io/aws-load-balancer-internal: "false" + service.beta.kubernetes.io/aws-load-balancer-subnets: subnet-priv-a,subnet-priv-b +spec: + type: LoadBalancer + ports: + - port: 80 diff --git a/policies/kyverno/tests/networking-adopt/resources.yaml b/policies/kyverno/tests/networking-adopt/resources.yaml index d6bdf82..dc1c969 100644 --- a/policies/kyverno/tests/networking-adopt/resources.yaml +++ b/policies/kyverno/tests/networking-adopt/resources.yaml @@ -2,7 +2,7 @@ # live in a workload namespace (the policy matches every namespace). The # network-config ConfigMap state is supplied to the CLI as context via values.yaml. --- -# Plain Ingress, no scheme annotation → defaults to internal → private subnets. +# Plain Ingress, no scheme annotation → the controller's default → private subnets. apiVersion: networking.k8s.io/v1 kind: Ingress metadata: @@ -37,7 +37,24 @@ spec: rules: - host: c.example.com --- -# Plain LoadBalancer Service, no scheme annotation → internal → private subnets. +# Ingress in a group. One load balancer serves every member and its scheme is +# decided across all of them, so this object does not carry the answer even +# though it looks like it does — no injection. +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: ing-grouped + namespace: workloads + annotations: + alb.ingress.kubernetes.io/group.name: shared +spec: + rules: + - host: d.example.com +--- +# Plain LoadBalancer Service: no loadBalancerClass, no aws-load-balancer-type. +# The AWS Load Balancer Controller does not claim it, the in-tree cloud provider +# builds a Classic Load Balancer, and neither the scheme nor the subnets +# annotation is read — so injecting one would be writing to nobody. apiVersion: v1 kind: Service metadata: @@ -48,19 +65,94 @@ spec: ports: - port: 80 --- -# LoadBalancer Service explicitly internet-facing → public subnets. +# Claimed by loadBalancerClass, no scheme stated → default → private subnets. +apiVersion: v1 +kind: Service +metadata: + name: svc-nlb-class + namespace: workloads +spec: + type: LoadBalancer + loadBalancerClass: service.k8s.aws/nlb + ports: + - port: 80 +--- +# Claimed by the type annotation plus an instance target type → private subnets. +apiVersion: v1 +kind: Service +metadata: + name: svc-external-instance + namespace: workloads + annotations: + service.beta.kubernetes.io/aws-load-balancer-type: external + service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: instance +spec: + type: LoadBalancer + ports: + - port: 80 +--- +# Claimed by the nlb-ip type annotation, explicitly internet-facing → public. apiVersion: v1 kind: Service metadata: name: svc-internet namespace: workloads annotations: + service.beta.kubernetes.io/aws-load-balancer-type: nlb-ip service.beta.kubernetes.io/aws-load-balancer-scheme: internet-facing spec: type: LoadBalancer ports: - port: 80 --- +# The legacy spelling with no scheme annotation. "false" means internet-facing to +# the controller, so this Service needs the PUBLIC subnets. Reading only the +# scheme annotation makes it look like a default-internal Service and hands it +# the private CSV for a load balancer the controller puts on public subnets. +apiVersion: v1 +kind: Service +metadata: + name: svc-legacy-internal-false + namespace: workloads + annotations: + service.beta.kubernetes.io/aws-load-balancer-type: external + service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: ip + service.beta.kubernetes.io/aws-load-balancer-internal: "false" +spec: + type: LoadBalancer + ports: + - port: 80 +--- +# The same legacy spelling saying internal → private subnets. +apiVersion: v1 +kind: Service +metadata: + name: svc-legacy-internal-true + namespace: workloads + annotations: + service.beta.kubernetes.io/aws-load-balancer-internal: "true" +spec: + type: LoadBalancer + loadBalancerClass: service.k8s.aws/nlb + ports: + - port: 80 +--- +# Both spellings, disagreeing. The scheme annotation is read first and the legacy +# flag is not consulted, so this is internal and takes the private subnets. +apiVersion: v1 +kind: Service +metadata: + name: svc-scheme-beats-legacy + namespace: workloads + annotations: + service.beta.kubernetes.io/aws-load-balancer-type: nlb-ip + service.beta.kubernetes.io/aws-load-balancer-scheme: internal + service.beta.kubernetes.io/aws-load-balancer-internal: "false" +spec: + type: LoadBalancer + ports: + - port: 80 +--- # A ClusterIP Service is not a load balancer → the spec.type precondition skips it, # proving the mutation does not touch non-LoadBalancer Services. apiVersion: v1 diff --git a/scripts/check-lb-scheme-inputs.py b/scripts/check-lb-scheme-inputs.py new file mode 100755 index 0000000..8ef9343 --- /dev/null +++ b/scripts/check-lb-scheme-inputs.py @@ -0,0 +1,607 @@ +#!/usr/bin/env python3 +"""Every input that decides a load balancer's scheme is one this catalog's policy reads. + + python3 scripts/check-lb-scheme-inputs.py # blocking gate, offline + python3 scripts/check-lb-scheme-inputs.py --live # scheduled, reads the controller + python3 scripts/check-lb-scheme-inputs.py --sync # re-derive and rewrite the record + python3 scripts/check-lb-scheme-inputs.py --self-test + +WHY A GATE AND NOT A LONGER LIST + +inject-adopt-lb-subnets injects private or public subnet ids according to the +scheme it believes a load balancer will have. Reading one annotation per object +kind made that belief a pattern rather than a derivation: a second spelling of +the same thing — `aws-load-balancer-internal`, still honoured, still ahead of the +default — read as internal and handed a private-subnet list to a load balancer +the controller puts on public subnets. Adding that second annotation to the +policy fixes the instance. It does nothing about the third. + +So the population is derived from the code that actually decides it. The AWS Load +Balancer Controller answers two questions, in two functions, and this gate reads +both at the version the catalog pins: + + buildLoadBalancerScheme (pkg/service and pkg/ingress) — which scheme + IsServiceSupported (pkg/service) — whose Service it is + +Every symbol those functions and their callees consult is derived and must be +accounted for in scripts/lb-scheme-inputs.json: READ, with a string the policy +must contain; UNREAD, with the reason the policy cannot or will not consult it; +or PLUMBING, with the reason it decides nothing. A symbol that is none of those +fails --sync, so a new deciding input cannot be recorded without somebody +choosing what to do about it. + +WHAT THIS DOES NOT ESTABLISH + +That the derivation sees every possible input. It reads the two entry functions +and the functions they call within their own files; an input reached through a +package this walk does not open would be missed. What it does establish is that +the set is derived from the controller rather than remembered, and that the set +moving fails a build. + +THE SPLIT + + default (offline, BLOCKING) — the record matches the chart pin, every symbol + recorded READ appears in the policy, every other symbol carries its + reason, and every literal the policy hardcodes is one the record derived. + A function of the tree, so a chart bump makes the record stale and fails + here rather than on a cluster. + + --live (network, SCHEDULED) — re-derive the symbols from the controller + source, and re-render the chart with this catalog's values to confirm the + flags the policy assumes are the flags the controller is given. +""" + +from __future__ import annotations + +import argparse +import importlib.util +import json +import pathlib +import re +import subprocess +import sys +import urllib.error +import urllib.request +from typing import NoReturn + +# Shared precondition helper, loaded by path: these are hyphenated executables +# run from varying working directories. +_gl = pathlib.Path(__file__).resolve().parent / "gatelib.py" +_gs = importlib.util.spec_from_file_location("gatelib", _gl) +assert _gs and _gs.loader, f"{_gl} is not loadable as a module" +gatelib = importlib.util.module_from_spec(_gs) +sys.modules["gatelib"] = gatelib +_gs.loader.exec_module(gatelib) + +ROOT = pathlib.Path(__file__).resolve().parent.parent +APPSET = ROOT / "applicationsets" / "addons-networking.yaml" +ADDON = ROOT / "addons" / "networking" / "aws-load-balancer-controller" +POLICY = ROOT / "policies" / "kyverno" / "networking" / "base" / "inject-adopt-lb-subnets.yaml" +# Beside the checker for the reason scripts/chart-provenance.json is: the +# directories it describes are read by other gates as manifests. +RECORDS = ROOT / "scripts" / "lb-scheme-inputs.json" + +ADDON_NAME = "aws-load-balancer-controller" +SOURCE = "https://raw.githubusercontent.com/kubernetes-sigs/aws-load-balancer-controller" + +# The functions that decide, and the file each lives in. Entry points, not a list +# of inputs — what they consult is derived. +ENTRY_POINTS = ( + ("scheme", "Service", "pkg/service/model_build_load_balancer.go", "buildLoadBalancerScheme"), + ("scheme", "Ingress", "pkg/ingress/model_build_load_balancer.go", "buildLoadBalancerScheme"), + ("ownership", "Service", "pkg/service/service_utils.go", "IsServiceSupported"), +) +# Read for the annotation names and prefixes the entry points reference by +# identifier, and for the string values the policy has to match. +LOOKUP_FILES = ( + "pkg/annotations/constants.go", + "pkg/service/model_builder.go", + "controllers/service/service_controller.go", +) + +STATUSES = ("read", "unread", "plumbing") +NETWORK_TIMEOUT = 120 + + +def die(msg: str) -> NoReturn: + print(f"lb-scheme-inputs: {msg}", file=sys.stderr) + sys.exit(1) + + +# ------------------------------------------------------------------- the chart + + +def chart_pin() -> dict: + """The controller's chart coordinates, from the ApplicationSet that ships it.""" + for doc in gatelib.read_yaml_all(APPSET): + if not isinstance(doc, dict) or doc.get("kind") != "ApplicationSet": + continue + for el in gatelib.list_elements(doc): + if el.get("appName") == ADDON_NAME: + missing = [k for k in ("chartRepo", "chart", "chartVersion") if not el.get(k)] + if missing: + die(f"{APPSET.relative_to(ROOT)} names {ADDON_NAME} without " + f"{', '.join(missing)} — the version this gate reads the " + f"controller at is not in the tree.") + return {"chartRepo": str(el["chartRepo"]), "chart": str(el["chart"]), + "chartVersion": str(el["chartVersion"])} + die(f"{APPSET.relative_to(ROOT)} carries no {ADDON_NAME} element, so this gate has " + f"no controller version to derive against. It examined nothing, which is not " + f"the same as finding nothing.") + raise AssertionError("unreachable") + + +def app_version(pin: dict) -> str: + """What the pinned chart says it installs — the tag the source is read at.""" + gatelib.require("helm") + proc = subprocess.run( + ["helm", "show", "chart", "--repo", pin["chartRepo"], pin["chart"], + "--version", pin["chartVersion"]], + capture_output=True, text=True, timeout=NETWORK_TIMEOUT) + if proc.returncode != 0: + last = ((proc.stderr or "") + (proc.stdout or "")).strip().splitlines() + die(f"{pin['chart']} {pin['chartVersion']} would not resolve — " + f"{last[-1][:200] if last else 'no output'}") + import yaml + meta = yaml.safe_load(proc.stdout) or {} + version = str(meta.get("appVersion") or "").strip() + if not version: + die(f"{pin['chart']} {pin['chartVersion']} publishes no appVersion, so which " + f"controller source to read is unknown.") + return version + + +def rendered_flags(pin: dict) -> dict: + """The controller's effective configuration, as this catalog installs it.""" + gatelib.require("helm") + values = [str(ADDON / "values.yaml")] + proc = subprocess.run( + ["helm", "template", ADDON_NAME, pin["chart"], "--repo", pin["chartRepo"], + "--version", pin["chartVersion"], "-n", "kube-system", + *sum((["-f", v] for v in values), []), "--set", "clusterName=fixture"], + capture_output=True, text=True, timeout=NETWORK_TIMEOUT) + if proc.returncode != 0: + last = ((proc.stderr or "") + (proc.stdout or "")).strip().splitlines() + die(f"the controller chart would not render with this catalog's values — " + f"{last[-1][:200] if last else 'no output'}") + import yaml + args: list[str] = [] + mutates_services = False + for doc in yaml.safe_load_all(proc.stdout): + if not isinstance(doc, dict): + continue + if doc.get("kind") == "Deployment": + for container in doc["spec"]["template"]["spec"]["containers"]: + args += list(container.get("args") or []) + if doc.get("kind") == "MutatingWebhookConfiguration": + for hook in doc.get("webhooks") or []: + if "service" in str(hook.get("name", "")): + mutates_services = True + flags = {} + for arg in args: + name, _, value = str(arg).lstrip("-").partition("=") + flags[name] = value + return {"args": flags, "serviceMutatorWebhook": mutates_services} + + +# ---------------------------------------------------------- reading the source + + +def fetch(ref: str, path: str) -> str: + url = f"{SOURCE}/{ref}/{path}" + try: + with urllib.request.urlopen(url, timeout=NETWORK_TIMEOUT) as resp: # noqa: S310 + return resp.read().decode("utf-8") + except (urllib.error.URLError, OSError) as exc: + die(f"could not read {path} at {ref} — {exc}. The controller source is what " + f"this gate derives from; without it nothing was checked.") + + +def function_body(src: str, name: str) -> str | None: + """One top-level function's body, or None when the file declares no such function. + + gofmt puts a top-level closing brace in column zero, which is what makes the + end of a multi-line function findable without parsing Go. It also permits a + body written on the signature line, and that shape has to be read too: an + entry point written that way is loud (the walk finds nothing and the gate + refuses), but a one-line HELPER would simply drop out of the walk and take + whatever it consults with it, leaving a shorter input list and a clean run. + """ + m = re.search(rf"^func \([^)]*\) {re.escape(name)}\(.*$", src, re.M) + if m is None: + return None + line = m.group(0) + if line.rstrip().endswith("}") and "{" in line: + return line[line.index("{") + 1:line.rstrip().rindex("}")] + rest = src[m.end():] + end = rest.find("\n}\n") + return rest[:end] if end != -1 else None + + +def reachable(src: str, entry: str) -> list[tuple[str, str]]: + """`entry` and every function in the same file it calls, transitively.""" + seen: set[str] = set() + todo = [entry] + out: list[tuple[str, str]] = [] + while todo: + name = todo.pop() + if name in seen: + continue + seen.add(name) + body = function_body(src, name) + if body is None: + continue + out.append((name, body)) + # Receiver-agnostic: a helper on a different receiver decides just as much. + todo += [c for c in re.findall(r"\b\w+\.(\w+)\(", body) if c not in seen] + return out + + +def constants(sources: dict[str, str]) -> dict[str, str]: + """Every `Name = "value"` the lookup files declare.""" + found: dict[str, str] = {} + for text in sources.values(): + for name, value in re.findall(r'^\s*(\w+)\s*=\s*"([^"]*)"\s*$', text, re.M): + found.setdefault(name, value) + return found + + +def symbols(ref: str) -> dict[str, dict]: + """Every symbol the deciding functions consult, keyed for the record.""" + lookups = {p: fetch(ref, p) for p in LOOKUP_FILES} + consts = constants(lookups) + prefixes = { + "Ingress": consts.get("AnnotationPrefixIngress", ""), + "Service": consts.get("serviceAnnotationPrefix", ""), + } + for kind, prefix in prefixes.items(): + if not prefix: + die(f"the controller source at {ref} declares no annotation prefix for " + f"{kind}, so a suffix constant cannot be turned into the annotation " + f"a policy would read.") + + found: dict[str, dict] = {} + for decides, kind, path, entry in ENTRY_POINTS: + src = fetch(ref, path) + walked = reachable(src, entry) + if not walked: + die(f"{path} at {ref} declares no {entry} — the function this gate derives " + f"from has moved or been renamed, and a walk that found it missing " + f"must not report that everything is accounted for.") + text = "\n".join(body for _, body in walked) + + for ident in sorted(set(re.findall(r"\bannotations\.(\w+)", text))): + suffix = consts.get(ident) + if suffix is None: + die(f"{entry} consults annotations.{ident}, which none of " + f"{', '.join(LOOKUP_FILES)} declares. The annotation it names " + f"cannot be resolved, so whether the policy reads it is unknown.") + name = suffix if "/" in suffix else f"{prefixes[kind]}/{suffix}" + found[f"{kind}.annotations.{ident}"] = { + "decides": decides, "kind": kind, "annotation": name} + for field in sorted(set(re.findall(r"\.Spec\.(\w+)", text))): + found[f"{kind}.Spec.{field}"] = { + "decides": decides, "kind": kind, "annotation": None} + for field in sorted(set(re.findall(r"\b[a-z]\.(\w+)\b(?!\()", text))): + found[f"{kind}.{entry}.{field}"] = { + "decides": decides, "kind": kind, "annotation": None} + return found + + +# ----------------------------------------------------------------- the records + + +def load_records() -> dict: + if not RECORDS.exists(): + die(f"{RECORDS.relative_to(ROOT)} does not exist. Run --sync to create it.") + return gatelib.read_json(RECORDS) + + +def mentions(policy: str, text: str) -> bool: + """True when `policy` names `text` as a whole token rather than as a prefix. + + A containment test reads `aws-load-balancer-internal` as present inside + `aws-load-balancer-internal-something-else`, so renaming an annotation the + policy consults would leave this gate green. Annotation names, JMESPath paths + and quoted literals all end at the same class of character, so both edges are + asserted against it. + """ + edge = r"[A-Za-z0-9._/-]" + return re.search(rf"(? str: + if not POLICY.is_file(): + print(f"Cannot run: {POLICY.relative_to(ROOT)} does not exist. This gate " + f"examined no policy, which is not the same as finding nothing wrong " + f"with one.") + sys.exit(gatelib.CANNOT_RUN) + return POLICY.read_text(encoding="utf-8") + + +# ---------------------------------------------------------------- offline gate + + +def check_offline(record: dict, pin: dict, policy: str) -> int: + problems: list[str] = [] + controller = record.get("controller") or {} + recorded_symbols = record.get("symbols") or {} + literals = record.get("literals") or {} + + for field, value in pin.items(): + if controller.get(field) != value: + problems.append( + f"the controller is pinned at {field}={value} and " + f"{RECORDS.relative_to(ROOT)} was derived at " + f"{controller.get(field)!r}. Everything below was read out of a " + f"different version of the controller. Run --sync.") + if not controller.get("sourceRef"): + problems.append( + f"{RECORDS.relative_to(ROOT)} names no sourceRef, so which controller " + f"source the symbols came from is unknown.") + + if not recorded_symbols: + die(f"{RECORDS.relative_to(ROOT)} records no symbol at all. A run over an " + f"empty derivation reports the same thing as a run over a complete one.") + + for decides, kind, _, _ in ENTRY_POINTS: + if not any(s.get("decides") == decides and s.get("kind") == kind + for s in recorded_symbols.values()): + problems.append( + f"nothing is recorded for what decides {kind} {decides}, so that " + f"question was derived from nothing.") + + for key, rec in sorted(recorded_symbols.items()): + status = rec.get("status") + if status not in STATUSES: + problems.append( + f"{key} is recorded with status {status!r}, which is not one of " + f"{', '.join(STATUSES)} — it has not been decided about.") + continue + if status == "read": + evidence = rec.get("evidence") or rec.get("annotation") + if not evidence: + problems.append( + f"{key} is recorded as read with nothing naming where the policy " + f"reads it, so the claim rests on the record agreeing with itself.") + elif not mentions(policy, evidence): + problems.append( + f"{key} is recorded as read, and {POLICY.relative_to(ROOT)} does " + f"not contain {evidence!r}. The policy stopped reading an input " + f"the controller still decides on.") + elif not rec.get("note"): + problems.append( + f"{key} is recorded as {status} with no reason. An input nobody read " + f"and nobody excused is the gap this gate exists to close.") + + if not literals: + problems.append( + f"{RECORDS.relative_to(ROOT)} records no literal, so the strings the " + f"policy compares against are held equal to nothing.") + for value, why in sorted(literals.items()): + if not mentions(policy, value): + problems.append( + f"{POLICY.relative_to(ROOT)} does not contain {value!r} ({why}). The " + f"policy and the controller disagree about a value one of them " + f"decides on.") + + counts: dict[str, int] = {} + for rec in recorded_symbols.values(): + counts[rec.get("status", "?")] = counts.get(rec.get("status", "?"), 0) + 1 + print(f" {len(recorded_symbols)} symbol(s) derived from " + f"{controller.get('sourceRef')}: " + + ", ".join(f"{n} {s}" for s, n in sorted(counts.items()))) + for key, rec in sorted(recorded_symbols.items()): + if rec.get("status") != "read": + print(f" {rec.get('status'):9} {key}") + + if problems: + print(f"FAIL {len(problems)} problem(s):") + for p in problems: + print(f" {p}") + return 1 + print(f"OK {len(recorded_symbols)} deciding symbol(s) and {len(literals)} " + f"literal(s) accounted for against the pinned controller.") + return 0 + + +# ------------------------------------------------------------------ live check + + +def check_live(record: dict, pin: dict) -> int: + controller = record.get("controller") or {} + ref = controller.get("sourceRef") or app_version(pin) + derived = symbols(ref) + recorded = record.get("symbols") or {} + problems = [] + + for key in sorted(set(derived) - set(recorded)): + problems.append( + f"{key} decides {derived[key]['kind']} {derived[key]['decides']} and is in " + f"no record. Run --sync and decide what the policy does about it.") + for key in sorted(set(recorded) - set(derived)): + problems.append( + f"{key} is recorded and the controller at {ref} no longer consults it — " + f"drop it, or find out what replaced it.") + for key in sorted(set(recorded) & set(derived)): + if recorded[key].get("annotation") != derived[key]["annotation"]: + problems.append( + f"{key} resolves to annotation {derived[key]['annotation']!r} and is " + f"recorded as {recorded[key].get('annotation')!r}.") + + flags = rendered_flags(pin) + expected = record.get("controllerConfig") or {} + for name, want in sorted(expected.get("args", {}).items()): + got = flags["args"].get(name, want if name not in flags["args"] else "") + if name in flags["args"] and got != want: + problems.append( + f"the chart renders --{name}={got!r} and the policy is written for " + f"{want!r}.") + if flags["serviceMutatorWebhook"] != expected.get("serviceMutatorWebhook"): + problems.append( + f"the chart renders serviceMutatorWebhook=" + f"{flags['serviceMutatorWebhook']} and the record says " + f"{expected.get('serviceMutatorWebhook')}. Which Services carry a " + f"loadBalancerClass depends on it.") + + if problems: + print(f"FAIL {len(problems)} problem(s) against the controller at {ref}:") + for p in problems: + print(f" {p}") + return 1 + print(f"OK {len(derived)} deciding symbol(s) at {ref} match the record, and the " + f"rendered controller matches the configuration the policy is written for.") + return 0 + + +# ------------------------------------------------------------------------ sync + + +def sync(record: dict, pin: dict) -> int: + ref = app_version(pin) + derived = symbols(ref) + prior = record.get("symbols") or {} + + undecided = [] + out: dict[str, dict] = {} + for key, rec in sorted(derived.items()): + was = prior.get(key) or {} + entry = dict(rec) + entry["status"] = was.get("status", "") + if was.get("evidence"): + entry["evidence"] = was["evidence"] + if was.get("note"): + entry["note"] = was["note"] + if entry["status"] not in STATUSES: + undecided.append(key) + out[key] = entry + if undecided: + die(f"the controller at {ref} consults {len(undecided)} symbol(s) nothing has " + f"decided about:\n " + "\n ".join(undecided) + + f"\nAdd each to {RECORDS.relative_to(ROOT)} with a status of " + f"{'/'.join(STATUSES)} and, unless it is read, the reason. A new deciding " + f"input is a decision, not a record to regenerate.") + + flags = rendered_flags(pin) + config = record.get("controllerConfig") or {} + config["args"] = {k: flags["args"].get(k, v) for k, v in (config.get("args") or {}).items()} + config["serviceMutatorWebhook"] = flags["serviceMutatorWebhook"] + + RECORDS.write_text(json.dumps({ + "_README": README, + "controller": {**pin, "sourceRef": ref}, + "controllerConfig": config, + "literals": record.get("literals") or {}, + "symbols": out, + }, indent=2) + "\n") + print(f"wrote {RECORDS.relative_to(ROOT)} ({len(out)} symbol(s) at {ref})") + return 0 + + +README = ( + "Every symbol the AWS Load Balancer Controller consults when it decides a load " + "balancer's scheme, and when it decides whether a Service is its own, derived " + "from the controller source at the version the chart pin installs. Each is READ " + "by policies/kyverno/networking/base/inject-adopt-lb-subnets.yaml with a string " + "the policy must contain, UNREAD with the reason the policy does not consult it, " + "or PLUMBING with the reason it decides nothing. A symbol that is none of those " + "fails --sync: a new deciding input is a decision somebody makes, not a record to " + "regenerate. Re-derive with scripts/check-lb-scheme-inputs.py --sync." +) + + +# ------------------------------------------------------------------- self-test + + +def self_test() -> int: + """Break each input the offline verdict rests on and confirm it is rejected.""" + import contextlib + import copy + import io + + record = load_records() + pin = chart_pin() + policy = policy_text() + + def run(r, p, t): + with contextlib.redirect_stdout(io.StringIO()): + return check_offline(r, p, t) + + key = sorted(k for k, v in (record.get("symbols") or {}).items() + if v.get("status") == "read")[0] + unread = sorted(k for k, v in (record.get("symbols") or {}).items() + if v.get("status") != "read")[0] + + breaks = [] + + r = copy.deepcopy(record) + r["symbols"][key]["evidence"] = "an-annotation-the-policy-does-not-read" + breaks.append(("an input recorded as read that the policy does not read", r, pin, policy)) + + r = copy.deepcopy(record) + r["symbols"][unread].pop("note", None) + breaks.append(("an input nobody read and nobody excused", r, pin, policy)) + + r = copy.deepcopy(record) + r["symbols"][unread]["status"] = "" + breaks.append(("an input with no decision recorded about it", r, pin, policy)) + + r = copy.deepcopy(record) + r["controller"]["chartVersion"] = "0.0.0-not-the-pin" + breaks.append(("a record derived from a version nothing pins", r, pin, policy)) + + r = copy.deepcopy(record) + r["controller"]["sourceRef"] = "" + breaks.append(("a record that does not say which source it read", r, pin, policy)) + + r = copy.deepcopy(record) + r["literals"] = {} + breaks.append(("a policy whose literals are held equal to nothing", r, pin, policy)) + + literal = sorted(record.get("literals") or {})[0] + breaks.append((f"a policy that stopped naming {literal!r}", + record, pin, policy.replace(literal, "something-else"))) + + failures = [] + for label, r, p, t in breaks: + if run(r, p, t) == 0: + failures.append(label) + print(f" ACCEPTED {label} <-- not caught") + else: + print(f" rejected {label}") + + if run(record, pin, policy) != 0: + failures.append("the shipped policy does not pass") + print(" ACCEPTED (control) the shipped policy is rejected") + else: + print(f" passed (control) the shipped policy, " + f"{len(record.get('symbols') or {})} symbol(s)") + + if failures: + print(f"\nFAIL {len(failures)} break(s) not caught.") + return 1 + print(f"\nOK all {len(breaks)} breaks rejected, and the shipped policy passes.") + return 0 + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + ap.add_argument("--live", action="store_true", + help="re-derive from the controller source and the rendered chart") + ap.add_argument("--sync", action="store_true", + help="rewrite the record from the pinned controller (network)") + ap.add_argument("--self-test", action="store_true", + help="break the offline gate's inputs and confirm each is caught") + args = ap.parse_args(argv) + + if args.self_test: + return self_test() + if args.sync: + return sync(load_records() if RECORDS.exists() else {}, chart_pin()) + if args.live: + return check_live(load_records(), chart_pin()) + return check_offline(load_records(), chart_pin(), policy_text()) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/kyverno-test.sh b/scripts/kyverno-test.sh index 5b77633..683ee37 100755 --- a/scripts/kyverno-test.sh +++ b/scripts/kyverno-test.sh @@ -11,7 +11,7 @@ TESTS="${1:-policies/kyverno/tests}" # A floor on tests EXECUTED. Set under what the suite holds, so it catches # "matched almost nothing" rather than one case being retired. -MIN_TESTS=15 +MIN_TESTS=24 if ! command -v kyverno >/dev/null 2>&1; then echo "Cannot run: kyverno is not on PATH. No policy was tested — that is" @@ -19,12 +19,41 @@ if ! command -v kyverno >/dev/null 2>&1; then exit 2 fi +# A count is an operand, and an absent producer yields an EMPTY one, not a zero. +# `[ "" -gt 0 ]` exits 2 with "integer expected", and an `if` reads exit 2 as +# false — so the check is not failed, it is SKIPPED, and execution falls through +# to the pass. +require_count() { + case "$2" in + "" ) echo "Cannot run: $1 produced no count — its producer did not run. An" + echo "undetermined count is not a count of zero." + exit 2 ;; + *[!0-9]* ) echo "Cannot run: $1 produced a non-numeric count ($2)." + exit 2 ;; + esac +} + cd "$ROOT" out="$(kyverno test "$TESTS" 2>&1)" rc=$? printf '%s\n' "$out" [ "$rc" -ne 0 ] && exit "$rc" +# A policy the engine REJECTS is reported as "Invalid Policy", and a rejected +# policy skips every rule — which is what every row expecting a skip asserts. The +# CLI counts those rows as passes, so a variable expression Kyverno will not +# accept prints the same summary as a suite that evaluated everything. Only rows +# asserting a patched resource notice; a test file made of skips asserts nothing +# at all. +invalid="$(printf '%s' "$out" | grep -c 'Invalid Policy')" +require_count "the invalid-policy scan" "$invalid" +if [ "$invalid" -gt 0 ]; then + echo "FAIL kyverno reported $invalid result(s) against a policy it will not accept." + echo " A rejected policy skips every rule, which satisfies every expected" + echo " skip in the suite — the run below passed without evaluating anything." + exit 1 +fi + ran="$(printf '%s' "$out" | sed -n 's/^Test Summary: \([0-9][0-9]*\) tests passed.*/\1/p' | head -1)" if [ -z "$ran" ]; then echo "Cannot run: kyverno printed no test summary, so how many tests ran is" diff --git a/scripts/lb-scheme-inputs.json b/scripts/lb-scheme-inputs.json new file mode 100644 index 0000000..c59c249 --- /dev/null +++ b/scripts/lb-scheme-inputs.json @@ -0,0 +1,177 @@ +{ + "_README": "Every symbol the AWS Load Balancer Controller consults when it decides a load balancer's scheme, and when it decides whether a Service is its own, derived from the controller source at the version the chart pin installs. Each is READ by policies/kyverno/networking/base/inject-adopt-lb-subnets.yaml with a string the policy must contain, UNREAD with the reason the policy does not consult it, or PLUMBING with the reason it decides nothing. A symbol that is none of those fails --sync: a new deciding input is a decision somebody makes, not a record to regenerate. Re-derive with scripts/check-lb-scheme-inputs.py --sync.", + "controller": { + "chartRepo": "https://aws.github.io/eks-charts", + "chart": "aws-load-balancer-controller", + "chartVersion": "3.5.0", + "sourceRef": "v3.5.0" + }, + "controllerConfig": { + "args": { + "default-load-balancer-scheme": "internal", + "load-balancer-class": "service.k8s.aws/nlb", + "ingress-class": "alb" + }, + "serviceMutatorWebhook": false + }, + "literals": { + "'service.k8s.aws/nlb'": "the load balancer class the controller claims, --load-balancer-class", + "'nlb-ip'": "an aws-load-balancer-type value that claims a Service on its own", + "'external'": "the aws-load-balancer-type value that claims a Service with a target type", + "'ip'": "an aws-load-balancer-nlb-target-type value that completes the external claim", + "'instance'": "the other aws-load-balancer-nlb-target-type value that completes it", + "'internal'": "the scheme the controller falls back to, --default-load-balancer-scheme", + "'internet-facing'": "the other scheme the controller can compute" + }, + "symbols": { + "Ingress.Spec.Scheme": { + "decides": "scheme", + "kind": "Ingress", + "annotation": null, + "status": "unread", + "note": "IngressClassParams.spec.scheme overrides the object's own annotation, and reaching it means resolving the Ingress class and then its parameters. Kyverno evaluates a rule's context before its preconditions, so those two lookups would run on every Ingress admission on every cluster in the fleet, adopt or not, and Kyverno's admission controller is granted neither resource. An Ingress whose class parameters set a scheme and whose object states none therefore takes the default here; the controller rejects the mismatch at provisioning, which is the same loud failure an adopt cluster had before any injection existed." + }, + "Ingress.annotations.IngressSuffixScheme": { + "decides": "scheme", + "kind": "Ingress", + "annotation": "alb.ingress.kubernetes.io/scheme", + "status": "read" + }, + "Ingress.buildLoadBalancerScheme.annotationParser": { + "decides": "scheme", + "kind": "Ingress", + "annotation": null, + "status": "plumbing", + "note": "The reader that turns a suffix into an annotation lookup. It decides nothing; what it is asked for is recorded above." + }, + "Ingress.buildLoadBalancerScheme.defaultScheme": { + "decides": "scheme", + "kind": "Ingress", + "annotation": null, + "status": "read", + "evidence": "'internal'" + }, + "Ingress.buildLoadBalancerScheme.ingGroup": { + "decides": "scheme", + "kind": "Ingress", + "annotation": null, + "status": "unread", + "note": "One load balancer serves an Ingress group and one scheme is decided across every member, so no single object carries the answer. Rather than guess from the member being admitted, the policy declines to inject when alb.ingress.kubernetes.io/group.name is set." + }, + "Service.IsServiceSupported.annotationParser": { + "decides": "ownership", + "kind": "Service", + "annotation": null, + "status": "plumbing", + "note": "The reader that turns a suffix into an annotation lookup. It decides nothing; what it is asked for is recorded above." + }, + "Service.IsServiceSupported.featureGates": { + "decides": "ownership", + "kind": "Service", + "annotation": null, + "status": "unread", + "note": "The ServiceTypeLoadBalancerOnly gate only narrows which Services the controller claims. The policy requires spec.type LoadBalancer unconditionally, so its population is never wider than the controller's whichever way the gate is set." + }, + "Service.IsServiceSupported.loadBalancerClass": { + "decides": "ownership", + "kind": "Service", + "annotation": null, + "status": "read", + "evidence": "'service.k8s.aws/nlb'" + }, + "Service.Spec.LoadBalancerClass": { + "decides": "ownership", + "kind": "Service", + "annotation": null, + "status": "read", + "evidence": "request.object.spec.loadBalancerClass" + }, + "Service.Spec.Type": { + "decides": "ownership", + "kind": "Service", + "annotation": null, + "status": "read", + "evidence": "request.object.spec.type" + }, + "Service.annotations.SvcLBSuffixInternal": { + "decides": "scheme", + "kind": "Service", + "annotation": "service.beta.kubernetes.io/aws-load-balancer-internal", + "status": "read" + }, + "Service.annotations.SvcLBSuffixLoadBalancerType": { + "decides": "ownership", + "kind": "Service", + "annotation": "service.beta.kubernetes.io/aws-load-balancer-type", + "status": "read" + }, + "Service.annotations.SvcLBSuffixScheme": { + "decides": "scheme", + "kind": "Service", + "annotation": "service.beta.kubernetes.io/aws-load-balancer-scheme", + "status": "read" + }, + "Service.annotations.SvcLBSuffixTargetType": { + "decides": "ownership", + "kind": "Service", + "annotation": "service.beta.kubernetes.io/aws-load-balancer-nlb-target-type", + "status": "read" + }, + "Service.buildLoadBalancerScheme.annotationParser": { + "decides": "scheme", + "kind": "Service", + "annotation": null, + "status": "plumbing", + "note": "The reader that turns a suffix into an annotation lookup. It decides nothing; what it is asked for is recorded above." + }, + "Service.buildLoadBalancerScheme.defaultLoadBalancerScheme": { + "decides": "scheme", + "kind": "Service", + "annotation": null, + "status": "read", + "evidence": "'internal'" + }, + "Service.buildLoadBalancerScheme.elbv2TaggingManager": { + "decides": "scheme", + "kind": "Service", + "annotation": null, + "status": "plumbing", + "note": "Performs that same lookup against the AWS API." + }, + "Service.buildLoadBalancerScheme.existingLoadBalancer": { + "decides": "scheme", + "kind": "Service", + "annotation": null, + "status": "unread", + "note": "When neither annotation states a scheme the controller adopts the scheme of a load balancer that already exists, read from the AWS API by resource tag. An admission request carries no such thing and Kyverno has no route to one, so a Service whose scheme lives only in AWS takes the default here." + }, + "Service.buildLoadBalancerScheme.fetchExistingLoadBalancerOnce": { + "decides": "scheme", + "kind": "Service", + "annotation": null, + "status": "plumbing", + "note": "Memoises that same lookup for one reconcile." + }, + "Service.buildLoadBalancerScheme.service": { + "decides": "scheme", + "kind": "Service", + "annotation": null, + "status": "plumbing", + "note": "The Service being reconciled \u2014 the object the annotations above are read from." + }, + "Service.buildLoadBalancerScheme.stack": { + "decides": "scheme", + "kind": "Service", + "annotation": null, + "status": "plumbing", + "note": "The resource stack whose tags address the existing load balancer. It carries no scheme of its own; the lookup it serves is recorded as unread." + }, + "Service.buildLoadBalancerScheme.trackingProvider": { + "decides": "scheme", + "kind": "Service", + "annotation": null, + "status": "plumbing", + "note": "Builds the tag filter for that same lookup." + } + } +} diff --git a/scripts/tests/controls.py b/scripts/tests/controls.py index 77b6369..c53904b 100755 --- a/scripts/tests/controls.py +++ b/scripts/tests/controls.py @@ -550,6 +550,26 @@ def m_directory_manifest_size(root): marker='"maxCombinedDirectoryManifestsSize": "10M"') +def m_lb_scheme_inputs(root): + """Stop the policy reading the legacy scheme annotation. + + Which is the defect as it was: aws-load-balancer-internal is still honoured + by the controller and still ahead of its default, so a Service setting it and + nothing else is internet-facing to the controller and internal to a policy + that reads only the newer spelling — the private-subnet list on a load + balancer the controller puts on public subnets. Renamed rather than deleted, + because a policy that reads an annotation nobody sets is the same silence. + """ + rel = "policies/kyverno/networking/base/inject-adopt-lb-subnets.yaml" + path = root / rel + before = path.read_text() + marker = f"aws-load-balancer-internal-{MARKER}" + after = before.replace("aws-load-balancer-internal", marker) + assert after != before, f"{rel} carries no legacy scheme annotation to rename" + path.write_text(after) + return path, before, after, marker + + def m_chart_deprecation(root): """A recorded chart that nothing pins, which the offline gate must reject.""" import json @@ -632,6 +652,8 @@ def m_env_coverage(root): "check-directory-manifest-size.py": ("a directory source over the ceiling the " "repo-server is configured for", m_directory_manifest_size), + "check-lb-scheme-inputs.py": ("a scheme the controller decides on that the policy " + "stopped reading", m_lb_scheme_inputs), } diff --git a/scripts/tests/reverify-gates.sh b/scripts/tests/reverify-gates.sh index bbfb820..7519372 100755 --- a/scripts/tests/reverify-gates.sh +++ b/scripts/tests/reverify-gates.sh @@ -161,6 +161,7 @@ run 0 "check-env-coverage.py" ./scripts/check-env-coverage.py run 0 "check-burn-rate-budgets.py" ./scripts/check-burn-rate-budgets.py run 0 "check-secret-store-refs.py" ./scripts/check-secret-store-refs.py run 0 "check-directory-manifest-size.py" ./scripts/check-directory-manifest-size.py +run 0 "check-lb-scheme-inputs.py" ./scripts/check-lb-scheme-inputs.py run 0 "check-named-things.py" ./scripts/check-named-things.py run 0 "check-policy-validity.py" ./scripts/check-policy-validity.py run 0 "no-placeholders.sh" ./scripts/no-placeholders.sh @@ -537,6 +538,74 @@ run nonzero "directory-manifest-size: a directory source nothing measured" \ ./scripts/check-directory-manifest-size.py res $F +# The defect as it was. aws-load-balancer-internal is still honoured by the +# controller and still ahead of its default, so a Service setting it and nothing +# else is internet-facing to the controller and internal to a policy reading only +# the newer spelling — the private-subnet list on a load balancer the controller +# puts on public subnets. +F=policies/kyverno/networking/base/inject-adopt-lb-subnets.yaml; mut $F +python3 - "$F" <<'PYQ' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +m=s.replace("|| (internalannotation == 'false' && 'internet-facing' || 'internal')", + "|| 'internal'") +assert m!=s, "mutation did not land" +p.write_text(m) +print(" dropped the legacy spelling from the scheme the policy computes") +PYQ +run nonzero "kyverno: a Service the controller calls internet-facing takes private subnets" \ + ./scripts/kyverno-test.sh +res $F + +# A policy the engine refuses is not a policy that passed. Every row expecting a +# rule to skip is satisfied by the rule never running, and the CLI counts those +# as passes. +F=policies/kyverno/networking/base/inject-adopt-lb-subnets.yaml; mut $F +python3 - "$F" <<'PYQ' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +m=s.replace("{{ lbclass == 'service.k8s.aws/nlb' || lbtype == 'nlb-ip' || " + "(lbtype == 'external' && (nlbtargettype == 'ip' || nlbtargettype == 'instance')) }}", + "{{ 'yes' == 'yes' }}") +assert m!=s, "mutation did not land" +p.write_text(m) +print(" made the policy one Kyverno will not accept") +PYQ +run nonzero "kyverno: a rejected policy reports every expected skip as a pass" \ + ./scripts/kyverno-test.sh +res $F + +# The policy stops reading an input the controller still decides on. Renamed +# rather than deleted: an annotation nobody sets is the same silence. +F=policies/kyverno/networking/base/inject-adopt-lb-subnets.yaml; mut $F +python3 - "$F" <<'PYQ' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +m=s.replace("aws-load-balancer-internal", "aws-load-balancer-private") +assert m!=s, "mutation did not land" +p.write_text(m) +print(" renamed the legacy annotation the policy reads") +PYQ +run nonzero "lb-scheme-inputs: an input the controller decides on goes unread" \ + ./scripts/check-lb-scheme-inputs.py +res $F + +# The shape a Renovate pull request has. Moving the chart pin moves the +# controller, and every symbol in the record was derived from the version that +# was there before. +F=applicationsets/addons-networking.yaml; mut $F +python3 - "$F" <<'PYQ' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +m=s.replace('chartVersion: "3.5.0"', 'chartVersion: "3.6.0"', 1) +assert m!=s, "mutation did not land" +p.write_text(m) +print(" moved the controller chart pin and left the derivation where it was") +PYQ +run nonzero "lb-scheme-inputs: the controller moves and the derivation does not" \ + ./scripts/check-lb-scheme-inputs.py +res $F + F=addons/bootstrap/cert-manager/values-hub.yaml; mut $F; rm -f $F run nonzero "env-coverage: deleted hub delta" ./scripts/check-env-coverage.py res $F @@ -649,7 +718,7 @@ echo "RESULT pass=$pass fail=$fail" # The harness owes the same assertion it demands of the gates: with every `run` # line deleted it would report pass=0 fail=0 and exit 0, which is a green run # over nothing checked. -MIN_CHECKS=52 +MIN_CHECKS=57 total=$((pass + fail)) if [ "$total" -lt "$MIN_CHECKS" ]; then echo "FAIL ran $total check(s), under the floor of $MIN_CHECKS — this harness" diff --git a/scripts/tests/run.py b/scripts/tests/run.py index fa78c6d..94e9c28 100755 --- a/scripts/tests/run.py +++ b/scripts/tests/run.py @@ -71,6 +71,9 @@ # The source-type decision that says which sources a size limit even applies # to, and the byte accounting the repo-server does once it has decided. "test_directory_manifest_size", + # The walk that derives which inputs decide a load balancer's scheme, and the + # two verdicts that make an input nobody thought about visible. + "test_lb_scheme_inputs", ) # A floor well under the real count. It catches "discovery found almost nothing", @@ -145,7 +148,7 @@ # largest in the tree, and among them are the gates on the paths testing-rubric # calls security-critical. So this figure being low is not offset by behavioural # coverage for precisely the files where that offset was being claimed. -COMBINED_FLOOR = 44 +COMBINED_FLOOR = 45 # A ceiling on gate scripts carrying NO unit coverage at all, complementing the # floors below. The floors stop a covered file regressing; nothing stopped a NEW @@ -190,6 +193,9 @@ # The half of the directory-size gate that decides the verdict. The rest is # `--sync`, `--live` and `--self-test`, which run against a clone. "scripts/check-directory-manifest-size.py": 70, + # The Go reading and the offline verdict. `--sync`, `--live` and the chart + # render reach the network and are covered by stubbing what they return. + "scripts/check-lb-scheme-inputs.py": 65, } diff --git a/scripts/tests/test_lb_scheme_inputs.py b/scripts/tests/test_lb_scheme_inputs.py new file mode 100644 index 0000000..ff68f69 --- /dev/null +++ b/scripts/tests/test_lb_scheme_inputs.py @@ -0,0 +1,561 @@ +"""Unit tests for the load-balancer scheme-input gate. + +The gate's whole value is that the set of inputs is DERIVED from the controller +rather than remembered, so the tests concentrate on the derivation and on the two +verdicts that make a new input visible: a symbol the controller consults and the +record does not carry, and a symbol the record carries that the policy has +stopped reading. + +The Go reading is deliberately small — find a function, follow the calls it makes +within its own file, collect what those bodies name — and small is only safe if +each clause is pinned. A walk that silently stops one call short produces a +shorter input list and a clean run, which is the failure this gate exists to +prevent rather than to demonstrate. +""" + +from __future__ import annotations + +import contextlib +import io +import json +import pathlib +import tempfile +import unittest + +from gateloader import load + +gate = load("check-lb-scheme-inputs") + +ROOT = pathlib.Path(__file__).resolve().parent.parent.parent + +PIN = {"chartRepo": "https://example.invalid/charts", "chart": "lbc", "chartVersion": "1.2.3"} + + +def record(symbols=None, literals=None, **kw): + doc = { + "controller": {**PIN, "sourceRef": "v1.2.3"}, + "controllerConfig": {"args": {"default-load-balancer-scheme": "internal"}, + "serviceMutatorWebhook": False}, + "literals": literals if literals is not None else {"'internal'": "the default"}, + "symbols": symbols if symbols is not None else { + "Service.annotations.SvcLBSuffixScheme": { + "decides": "scheme", "kind": "Service", + "annotation": "service.beta.kubernetes.io/aws-load-balancer-scheme", + "status": "read"}, + "Ingress.annotations.IngressSuffixScheme": { + "decides": "scheme", "kind": "Ingress", + "annotation": "alb.ingress.kubernetes.io/scheme", "status": "read"}, + "Service.Spec.LoadBalancerClass": { + "decides": "ownership", "kind": "Service", "annotation": None, + "status": "read", "evidence": "request.object.spec.loadBalancerClass"}, + "Service.buildLoadBalancerScheme.existingLoadBalancer": { + "decides": "scheme", "kind": "Service", "annotation": None, + "status": "unread", "note": "not visible from an admission request"}, + }, + } + doc.update(kw) + return doc + + +POLICY = ( + "service.beta.kubernetes.io/aws-load-balancer-scheme\n" + "alb.ingress.kubernetes.io/scheme\n" + "request.object.spec.loadBalancerClass\n" + "|| 'internal' }}\n" +) + + +def verdict(rec=None, pin=None, policy=None): + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + rc = gate.check_offline(rec or record(), pin or PIN, policy or POLICY) + return rc, buf.getvalue() + + +GO = '''package service + +func (t *task) buildLoadBalancerScheme(ctx context.Context) (Scheme, error) { + scheme, ok, err := t.viaAnnotation(ctx) + if ok { + return scheme, nil + } + return t.defaultLoadBalancerScheme, nil +} + +func (t *task) viaAnnotation(ctx context.Context) (Scheme, bool, error) { + if exists := t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixScheme, &raw, t.service.Annotations); exists { + return raw, true, nil + } + return t.legacy(ctx) +} + +func (t *task) legacy(_ context.Context) (Scheme, bool, error) { + _, err := t.annotationParser.ParseBoolAnnotation(annotations.SvcLBSuffixInternal, &internal, t.service.Annotations) + return SchemeInternal, false, err +} + +func (t *task) unrelated(ctx context.Context) error { + return t.annotationParser.Parse(annotations.SvcLBSuffixNotConsulted) +} +''' + + +class ReadingGoSource(unittest.TestCase): + def test_a_function_body_ends_at_the_closing_brace_in_column_zero(self): + body = gate.function_body(GO, "legacy") + self.assertIn("SvcLBSuffixInternal", body) + self.assertNotIn("unrelated", body) + + def test_a_function_that_is_not_there_is_not_an_empty_one(self): + # None, never "": a caller cannot tell an empty body from an absent + # function if both are falsy, and the absent one means the derivation + # lost its entry point. + self.assertIsNone(gate.function_body(GO, "noSuchFunction")) + + def test_the_walk_follows_calls_transitively(self): + names = [n for n, _ in gate.reachable(GO, "buildLoadBalancerScheme")] + self.assertEqual(sorted(names), ["buildLoadBalancerScheme", "legacy", "viaAnnotation"]) + + def test_the_walk_does_not_reach_what_the_entry_point_never_calls(self): + text = "\n".join(b for _, b in gate.reachable(GO, "buildLoadBalancerScheme")) + self.assertNotIn("SvcLBSuffixNotConsulted", text) + + def test_the_walk_terminates_on_a_cycle(self): + cyclic = GO + ''' +func (t *task) a(ctx context.Context) error { + return t.b(ctx) +} + +func (t *task) b(ctx context.Context) error { + return t.a(ctx) +} +''' + names = [n for n, _ in gate.reachable(cyclic, "a")] + self.assertEqual(sorted(names), ["a", "b"]) + + def test_a_body_written_on_the_signature_line_is_read(self): + # gofmt allows it, and a one-line helper dropping out of the walk takes + # whatever it consults with it — a shorter input list and a clean run. + oneline = '''package service + +func (t *task) entry(ctx context.Context) error { + return t.helper(ctx) +} + +func (t *task) helper(ctx context.Context) error { return t.parser.Parse(annotations.SvcLBSuffixOneLine) } +''' + text = "\n".join(b for _, b in gate.reachable(oneline, "entry")) + self.assertIn("SvcLBSuffixOneLine", text) + + def test_a_helper_on_another_receiver_is_followed_too(self): + # IsServiceSupported reaches its type check through a different receiver + # name; a walk keyed on one receiver drops half the ownership decision. + other = '''package service + +func (u *utils) IsServiceSupported(s *Service) bool { + return u.checkTypeAnnotation(s) +} + +func (u *utils) checkTypeAnnotation(s *Service) bool { + return u.annotationParser.Parse(annotations.SvcLBSuffixLoadBalancerType) +} +''' + text = "\n".join(b for _, b in gate.reachable(other, "IsServiceSupported")) + self.assertIn("SvcLBSuffixLoadBalancerType", text) + + def test_constants_are_read_as_name_to_value(self): + consts = gate.constants({"c.go": '''package annotations +const ( + AnnotationPrefixIngress = "alb.ingress.kubernetes.io" + IngressSuffixScheme = "scheme" + IngressClass = "kubernetes.io/ingress.class" + NotAString = 7 +) +'''}) + self.assertEqual(consts["IngressSuffixScheme"], "scheme") + self.assertEqual(consts["AnnotationPrefixIngress"], "alb.ingress.kubernetes.io") + self.assertNotIn("NotAString", consts) + + +class DerivingTheSymbols(unittest.TestCase): + CONSTS = '''package annotations +const ( + AnnotationPrefixIngress = "alb.ingress.kubernetes.io" + IngressSuffixScheme = "scheme" + IngressClass = "kubernetes.io/ingress.class" + SvcLBSuffixScheme = "aws-load-balancer-scheme" + SvcLBSuffixInternal = "aws-load-balancer-internal" + serviceAnnotationPrefix = "service.beta.kubernetes.io" +) +''' + INGRESS = '''package ingress + +func (t *task) buildLoadBalancerScheme(_ context.Context) (Scheme, error) { + if member.IngClassConfig.IngClassParams.Spec.Scheme != nil { + return *member.IngClassConfig.IngClassParams.Spec.Scheme, nil + } + t.annotationParser.ParseStringAnnotation(annotations.IngressSuffixScheme, &raw, member.Ing.Annotations) + return t.defaultScheme, nil +} +''' + OWNERSHIP = '''package service + +func (u *utils) IsServiceSupported(service *Service) bool { + if service.Spec.LoadBalancerClass != nil { + return *service.Spec.LoadBalancerClass == u.loadBalancerClass + } + return false +} +''' + + def derive(self): + files = {"pkg/annotations/constants.go": self.CONSTS, + "pkg/service/model_builder.go": "package service\n", + "controllers/service/service_controller.go": self.CONSTS, + "pkg/service/model_build_load_balancer.go": GO, + "pkg/ingress/model_build_load_balancer.go": self.INGRESS, + "pkg/service/service_utils.go": self.OWNERSHIP} + real = gate.fetch + gate.fetch = lambda ref, path: files[path] + try: + return gate.symbols("vtest") + finally: + gate.fetch = real + + def test_a_suffix_becomes_the_annotation_a_policy_would_read(self): + got = self.derive() + self.assertEqual(got["Service.annotations.SvcLBSuffixScheme"]["annotation"], + "service.beta.kubernetes.io/aws-load-balancer-scheme") + self.assertEqual(got["Ingress.annotations.IngressSuffixScheme"]["annotation"], + "alb.ingress.kubernetes.io/scheme") + + def test_the_legacy_spelling_is_derived_rather_than_listed(self): + # The defect that prompted the gate: a second annotation the controller + # honours, reached only because the walk follows the fallback call. + got = self.derive() + self.assertEqual(got["Service.annotations.SvcLBSuffixInternal"]["annotation"], + "service.beta.kubernetes.io/aws-load-balancer-internal") + + def test_a_source_that_is_not_an_annotation_is_derived_too(self): + got = self.derive() + self.assertIn("Ingress.Spec.Scheme", got) + self.assertIsNone(got["Ingress.Spec.Scheme"]["annotation"]) + self.assertIn("Service.Spec.LoadBalancerClass", got) + + def test_each_symbol_carries_the_question_it_answers(self): + got = self.derive() + self.assertEqual(got["Service.Spec.LoadBalancerClass"]["decides"], "ownership") + self.assertEqual(got["Ingress.Spec.Scheme"]["decides"], "scheme") + + def test_an_entry_point_that_has_moved_cannot_run(self): + files = {"pkg/annotations/constants.go": self.CONSTS, + "pkg/service/model_builder.go": "package service\n", + "controllers/service/service_controller.go": self.CONSTS, + "pkg/service/model_build_load_balancer.go": "package service\n", + "pkg/ingress/model_build_load_balancer.go": self.INGRESS, + "pkg/service/service_utils.go": self.OWNERSHIP} + real = gate.fetch + gate.fetch = lambda ref, path: files[path] + try: + with contextlib.redirect_stderr(io.StringIO()), self.assertRaises(SystemExit): + gate.symbols("vtest") + finally: + gate.fetch = real + + def test_an_annotation_constant_that_resolves_to_nothing_cannot_run(self): + files = {"pkg/annotations/constants.go": 'package annotations\nconst (\n\tAnnotationPrefixIngress = "alb.ingress.kubernetes.io"\n\tserviceAnnotationPrefix = "service.beta.kubernetes.io"\n)\n', + "pkg/service/model_builder.go": "package service\n", + "controllers/service/service_controller.go": "package service\n", + "pkg/service/model_build_load_balancer.go": GO, + "pkg/ingress/model_build_load_balancer.go": self.INGRESS, + "pkg/service/service_utils.go": self.OWNERSHIP} + real = gate.fetch + gate.fetch = lambda ref, path: files[path] + try: + with contextlib.redirect_stderr(io.StringIO()), self.assertRaises(SystemExit): + gate.symbols("vtest") + finally: + gate.fetch = real + + +class NamingSomethingIsNotContainingIt(unittest.TestCase): + """A prefix is not a mention, and reading it as one hides a rename.""" + + def test_a_whole_token_counts(self): + self.assertTrue(gate.mentions(POLICY, + "service.beta.kubernetes.io/aws-load-balancer-scheme")) + + def test_a_longer_annotation_does_not_count_as_the_shorter_one(self): + renamed = "service.beta.kubernetes.io/aws-load-balancer-internal-renamed" + self.assertFalse(gate.mentions(renamed, + "service.beta.kubernetes.io/aws-load-balancer-internal")) + + def test_a_literal_inside_a_longer_word_does_not_count(self): + self.assertFalse(gate.mentions("value: not-internal-either", "internal")) + self.assertTrue(gate.mentions("value: 'internal' }}", "'internal'")) + + +class TheOfflineVerdict(unittest.TestCase): + def test_the_shape_it_is_written_for_passes(self): + rc, said = verdict() + self.assertEqual(rc, 0, said) + + def test_an_input_the_policy_stopped_reading_is_rejected(self): + rc, said = verdict(policy=POLICY.replace("alb.ingress.kubernetes.io/scheme", "")) + self.assertEqual(rc, 1) + self.assertIn("IngressSuffixScheme", said) + + def test_an_input_read_by_something_other_than_its_annotation_name(self): + rc, said = verdict(policy=POLICY.replace("request.object.spec.loadBalancerClass", "")) + self.assertEqual(rc, 1) + self.assertIn("LoadBalancerClass", said) + + def test_an_input_recorded_read_with_nothing_naming_where(self): + rec = record() + rec["symbols"]["Service.Spec.LoadBalancerClass"].pop("evidence") + rc, said = verdict(rec) + self.assertEqual(rc, 1) + self.assertIn("agreeing with itself", said) + + def test_an_input_that_is_neither_read_nor_excused(self): + rec = record() + rec["symbols"]["Service.buildLoadBalancerScheme.existingLoadBalancer"].pop("note") + rc, said = verdict(rec) + self.assertEqual(rc, 1) + self.assertIn("nobody excused", said) + + def test_an_input_with_no_decision_recorded(self): + rec = record() + rec["symbols"]["Service.buildLoadBalancerScheme.existingLoadBalancer"]["status"] = "maybe" + rc, said = verdict(rec) + self.assertEqual(rc, 1) + self.assertIn("not been decided about", said) + + def test_a_record_derived_from_a_version_nothing_pins(self): + rec = record() + rec["controller"]["chartVersion"] = "9.9.9" + rc, said = verdict(rec) + self.assertEqual(rc, 1) + self.assertIn("different version", said) + + def test_a_record_that_does_not_say_which_source_it_read(self): + rec = record() + rec["controller"]["sourceRef"] = "" + rc, said = verdict(rec) + self.assertEqual(rc, 1) + + def test_a_derivation_with_no_symbols_at_all_is_not_a_pass(self): + with contextlib.redirect_stderr(io.StringIO()), self.assertRaises(SystemExit): + verdict(record(symbols={})) + + def test_a_question_nothing_was_derived_for_is_rejected(self): + rec = record(symbols={k: v for k, v in record()["symbols"].items() + if v["decides"] != "ownership"}) + rc, said = verdict(rec) + self.assertEqual(rc, 1) + self.assertIn("ownership", said) + + def test_a_literal_the_policy_no_longer_names(self): + rc, said = verdict(policy=POLICY.replace("|| 'internal' }}", "")) + self.assertEqual(rc, 1) + self.assertIn("'internal'", said) + + def test_literals_held_equal_to_nothing(self): + rc, said = verdict(record(literals={})) + self.assertEqual(rc, 1) + self.assertIn("held equal to nothing", said) + + +class TheLiveVerdict(unittest.TestCase): + """The half that finds the spelling nobody added.""" + + def live(self, derived, rec=None, flags=None): + real_sym, real_flags = gate.symbols, gate.rendered_flags + gate.symbols = lambda ref: derived + gate.rendered_flags = lambda pin: flags or { + "args": {"default-load-balancer-scheme": "internal"}, + "serviceMutatorWebhook": False} + try: + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + rc = gate.check_live(rec or record(), PIN) + return rc, buf.getvalue() + finally: + gate.symbols, gate.rendered_flags = real_sym, real_flags + + def derived_from(self, rec): + return {k: {"decides": v["decides"], "kind": v["kind"], + "annotation": v.get("annotation")} + for k, v in rec["symbols"].items()} + + def test_a_derivation_matching_the_record_passes(self): + rec = record() + rc, said = self.live(self.derived_from(rec), rec) + self.assertEqual(rc, 0, said) + + def test_a_deciding_input_the_record_does_not_carry_is_rejected(self): + # A third spelling appearing upstream: derived, unrecorded, and therefore + # unread by a policy nobody has thought about it for. + rec = record() + derived = self.derived_from(rec) + derived["Service.annotations.SvcLBSuffixSomethingNew"] = { + "decides": "scheme", "kind": "Service", + "annotation": "service.beta.kubernetes.io/aws-load-balancer-something-new"} + rc, said = self.live(derived, rec) + self.assertEqual(rc, 1) + self.assertIn("SomethingNew", said) + self.assertIn("--sync", said) + + def test_a_recorded_input_upstream_no_longer_consults(self): + rec = record() + derived = self.derived_from(rec) + derived.pop("Service.buildLoadBalancerScheme.existingLoadBalancer") + rc, said = self.live(derived, rec) + self.assertEqual(rc, 1) + self.assertIn("no longer consults", said) + + def test_an_annotation_that_was_renamed_upstream(self): + rec = record() + derived = self.derived_from(rec) + derived["Service.annotations.SvcLBSuffixScheme"]["annotation"] = \ + "service.beta.kubernetes.io/aws-lb-scheme" + rc, said = self.live(derived, rec) + self.assertEqual(rc, 1) + self.assertIn("aws-lb-scheme", said) + + def test_a_controller_flag_the_policy_is_not_written_for(self): + rec = record() + rc, said = self.live(self.derived_from(rec), rec, + flags={"args": {"default-load-balancer-scheme": "internet-facing"}, + "serviceMutatorWebhook": False}) + self.assertEqual(rc, 1) + self.assertIn("internet-facing", said) + + def test_the_service_mutator_webhook_coming_back(self): + # It stamps loadBalancerClass onto every LoadBalancer Service, which moves + # the whole plain shape into the controller's population. + rec = record() + rc, said = self.live(self.derived_from(rec), rec, + flags={"args": {"default-load-balancer-scheme": "internal"}, + "serviceMutatorWebhook": True}) + self.assertEqual(rc, 1) + self.assertIn("loadBalancerClass", said) + + +class WritingTheRecord(unittest.TestCase): + """--sync regenerates what was derived and refuses to invent a decision.""" + + def sync(self, derived, prior, flags=None): + # Inside the repo root: the gate names its record relative to it, and a + # path outside cannot be named that way. + real = (gate.symbols, gate.rendered_flags, gate.app_version, gate.RECORDS) + gate.symbols = lambda ref: derived + gate.app_version = lambda pin: "v1.2.3" + gate.rendered_flags = lambda pin: flags or { + "args": {"default-load-balancer-scheme": "internal"}, + "serviceMutatorWebhook": False} + try: + with tempfile.TemporaryDirectory(dir=ROOT) as tmp: + out = pathlib.Path(tmp) / "record.json" + out.write_text(json.dumps(prior)) + gate.RECORDS = out + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + rc = gate.sync(prior, PIN) + return rc, json.loads(out.read_text()), buf.getvalue() + finally: + gate.symbols, gate.rendered_flags, gate.app_version, gate.RECORDS = real + + def derived_from(self, rec): + return {k: {"decides": v["decides"], "kind": v["kind"], + "annotation": v.get("annotation")} + for k, v in rec["symbols"].items()} + + def test_a_symbol_nobody_decided_about_stops_the_sync(self): + # The property the gate exists for. A new deciding input is a decision + # somebody makes; regenerating the record past it would record silence. + prior = record() + derived = self.derived_from(prior) + derived["Service.annotations.SvcLBSuffixSomethingNew"] = { + "decides": "scheme", "kind": "Service", "annotation": "x/y"} + with contextlib.redirect_stderr(io.StringIO()) as err, self.assertRaises(SystemExit): + self.sync(derived, prior) + self.assertIn("SomethingNew", err.getvalue()) + + def test_a_decision_already_made_is_carried_forward(self): + prior = record() + rc, written, _ = self.sync(self.derived_from(prior), prior) + self.assertEqual(rc, 0) + was = prior["symbols"]["Service.buildLoadBalancerScheme.existingLoadBalancer"] + now = written["symbols"]["Service.buildLoadBalancerScheme.existingLoadBalancer"] + self.assertEqual(now["status"], was["status"]) + self.assertEqual(now["note"], was["note"]) + + def test_what_sync_writes_is_what_the_gate_compares_against(self): + # A generator and a comparison that disagree leave a record nothing can + # be regenerated to. + prior = record() + _, written, _ = self.sync(self.derived_from(prior), prior) + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + rc = gate.check_offline(written, PIN, POLICY) + self.assertEqual(rc, 0, buf.getvalue()) + + def test_the_source_it_read_is_recorded(self): + prior = record() + _, written, _ = self.sync(self.derived_from(prior), prior) + self.assertEqual(written["controller"]["sourceRef"], "v1.2.3") + self.assertEqual(written["controller"]["chartVersion"], PIN["chartVersion"]) + + def test_the_rendered_configuration_is_recorded_not_assumed(self): + prior = record() + _, written, _ = self.sync( + self.derived_from(prior), prior, + flags={"args": {"default-load-balancer-scheme": "internet-facing"}, + "serviceMutatorWebhook": True}) + self.assertEqual(written["controllerConfig"]["args"]["default-load-balancer-scheme"], + "internet-facing") + self.assertTrue(written["controllerConfig"]["serviceMutatorWebhook"]) + + +class TheShippedCatalog(unittest.TestCase): + def setUp(self): + self.record = gate.load_records() + self.pin = gate.chart_pin() + self.policy = gate.policy_text() + + def test_the_pin_is_read_out_of_the_applicationset(self): + self.assertEqual(self.pin["chart"], "aws-load-balancer-controller") + self.assertTrue(self.pin["chartVersion"]) + + def test_the_record_was_derived_at_the_pinned_version(self): + self.assertEqual(self.record["controller"]["chartVersion"], self.pin["chartVersion"]) + + def test_both_questions_are_derived_for(self): + decides = {(s["decides"], s["kind"]) for s in self.record["symbols"].values()} + self.assertIn(("scheme", "Service"), decides) + self.assertIn(("scheme", "Ingress"), decides) + self.assertIn(("ownership", "Service"), decides) + + def test_the_legacy_spelling_is_one_the_policy_reads(self): + entry = self.record["symbols"]["Service.annotations.SvcLBSuffixInternal"] + self.assertEqual(entry["status"], "read") + self.assertIn(entry["annotation"], self.policy) + + def test_every_unread_input_says_why(self): + for key, entry in self.record["symbols"].items(): + if entry["status"] != "read": + with self.subTest(key=key): + self.assertTrue(entry.get("note")) + + def test_the_catalog_passes_its_own_gate(self): + rc, said = verdict(self.record, self.pin, self.policy) + self.assertEqual(rc, 0, said) + + def test_the_record_is_what_sync_would_write(self): + doc = json.loads((ROOT / "scripts" / "lb-scheme-inputs.json").read_text()) + self.assertEqual(doc["_README"], gate.README) + + +if __name__ == "__main__": + unittest.main()