Skip to content

KNOX-3352: k8s pre-auth validator for service account annotation#1268

Merged
smolnar82 merged 2 commits into
apache:masterfrom
hanicz:KNOX-3353
Jun 18, 2026
Merged

KNOX-3352: k8s pre-auth validator for service account annotation#1268
smolnar82 merged 2 commits into
apache:masterfrom
hanicz:KNOX-3353

Conversation

@hanicz

@hanicz hanicz commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

KNOX-3353 - k8s pre-auth validator for service account annotation

What changes were proposed in this pull request?

  • New provider to pre-validate requests with k8s service account annotations
  • Service Account is provided with spiffe id
  • Default spiffe-id header: x-spiffe-id
  • Default user annotation header: x-knoxidf-obo.username
  • Default cache TTL: 60 seconds

How was this patch tested?

Unit tests, manual tests on local kind cluster

ServiceAccount

apiVersion: v1
kind: ServiceAccount
metadata:
  name: test-sa
  namespace: test
  annotations:
    knox.apache.org/owner-username: "bob"

RBAC

apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
  name: knox-sa-reader
  namespace: test
rules:
  - apiGroups: [""]
    resources: ["serviceaccounts"]
    verbs: ["get", "list"]
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
  name: knox-sa-reader
  namespace: test
subjects:
  - kind: ServiceAccount
    name: knox
    namespace: knox
roleRef:
  kind: Role
  name: knox-sa-reader
  apiGroup: rbac.authorization.k8s.io

health.xml

<topology>
    <gateway>
        <provider>
            <role>federation</role>
            <name>K8sPreAuth</name>
            <enabled>true</enabled>
            <param><name>preauth.custom.header</name><value>x-knoxidf-obo.username</value></param>
            <param><name>preauth.spiffe.header</name><value>x-spiffe-id</value></param>
            <param><name>preauth.k8s.user.annotation</name><value>knox.apache.org/owner-username</value></param>
        </provider>
    </gateway>
    <service>
        <role>HEALTH</role>
    </service>
</topology>

200 path:

curl -H 'x-spiffe-id: spiffe://cluster.local/ns/test/sa/test-sa' \
     -H 'x-knoxidf-obo.username: bob' \
     http://localhost:8443/gateway/health/v1/gateway-status

403 path:

curl -H 'x-spiffe-id: spiffe://cluster.local/ns/test/sa/test-sa' \
     -H 'x-knoxidf-obo.username: bobby' \
     http://localhost:8443/gateway/health/v1/gateway-status

< HTTP/1.1 403 Forbidden
HTTP/1.1 403 Forbidden
< Cache-Control: must-revalidate,no-cache,no-store
Cache-Control: must-revalidate,no-cache,no-store
< Content-Type: text/html;charset=iso-8859-1
Content-Type: text/html;charset=iso-8859-1
< Content-Length: 664
Content-Length: 664
<

<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"/>
<title>Error 403 Kubernetes pre-authentication failed: SPIFFE/ServiceAccount validation rejected the request.</title>
</head>
<body><h2>HTTP ERROR 403 Kubernetes pre-authentication failed: SPIFFE/ServiceAccount validation rejected the request.</h2>
<table>
<tr><th>URI:</th><td>/gateway/health/v1/gateway-status</td></tr>
<tr><th>STATUS:</th><td>403</td></tr>
<tr><th>MESSAGE:</th><td>Kubernetes pre-authentication failed: SPIFFE/ServiceAccount validation rejected the request.</td></tr>
<tr><th>SERVLET:</th><td>health-knox-gateway-servlet</td></tr>
</table>

</body>
</html>

Integration Tests

N/A

UI changes

N/A

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown

Test Results

22 tests   22 ✅  1s ⏱️
 1 suites   0 💤
 1 files     0 ❌

Results for commit 193073f.

@smolnar82 smolnar82 merged commit 755c4bc into apache:master Jun 18, 2026
3 checks passed
@smolnar82 smolnar82 mentioned this pull request Jun 18, 2026
Merged
@moresandeep moresandeep requested review from moresandeep and removed request for moresandeep June 18, 2026 13:26
@moresandeep

moresandeep commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Do we need to specify the error message details?

Error 403 Kubernetes pre-authentication failed: SPIFFE/ServiceAccount validation rejected the request.</title>

I think this is too much info for 403.

Also, I strongly think we should add integration tests for this.

@smolnar82

smolnar82 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Also, I strongly think we should add integration tests for this.

I couldn't agree more; see #1271 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smolnar82 smolnar82 smolnar82 approved these changes

@lmccay lmccay Awaiting requested review from lmccay

@pzampino pzampino Awaiting requested review from pzampino

@moresandeep moresandeep Awaiting requested review from moresandeep

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hanicz @moresandeep @smolnar82