Skip to content

Commit 997455b

Browse files
author
rzisholz
committed
Add provider_type segment to onboarding branches, fix configmap/tests/README
Cross-PR review caught real bugs missed by unit tests on both sides: - onboard-disco-agent.sh created per-cluster branches at data/<cluster-UUID>, but the authorizer's agent_provider_from_identity() expects host/data/<provider_type>/<provider_id>/workloads/<sub>. Real onboarded agents produced an identity the authorizer's own parser rejects. Fixed by adding a data/<type> branch before the cluster branch (now C1-C7, was C1-C6); verified end-to-end against the real authorizer parser. - templates/configmap.yaml never rendered the cyberark: block into the agent's config.yaml, despite values.yaml/values.schema.json declaring those fields — Conjur-JWT auth was silently unreachable via this chart. - TestConfig_CyberArk_Validation had two subtests asserting a config-time "service_id is required" error that no longer exists (validation moved to runtime, ErrNoAuthMethod). Updated to assert the current contract. - README's onboarding section documented a stale v1 policy-file mechanism that doesn't match what the v2-API script does, and never mentioned the script exists. Rewrote to point at hack/onboard-disco-agent.sh. Also fixed a self-contradicting serviceId example in both the README table and values.schema.json (prose said "bare segment", the example showed the full policy path).
1 parent 4a03eb8 commit 997455b

5 files changed

Lines changed: 83 additions & 93 deletions

File tree

deploy/charts/disco-agent/README.md

Lines changed: 29 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ CyberArk-hosted tenants:
8282

8383
| Value | Default | Description |
8484
|---|---|---|
85-
| `config.cyberark.serviceId` | `""` | Conjur authn-jwt service ID (required). Example: `conjur/authn-jwt/disco-agent` |
85+
| `config.cyberark.serviceId` | `""` | Conjur authn-jwt service ID (required). Example: `disco-agent` |
8686
| `config.cyberark.account` | `conjur` | Conjur account name. Always `conjur` for CyberArk-hosted tenants. |
8787
| `config.cyberark.jwtSource` | `file` | Token source. `file` = projected SA-token volume (default). `spiffe` deferred. |
8888
| `config.cyberark.jwtFilePath` | `/var/run/secrets/tokens/jwt` | Path to the projected token file. Auto-mounted by the chart when `jwtSource=file`. |
@@ -94,68 +94,33 @@ configuration is required.
9494

9595
### Per-tenant Conjur onboarding
9696

97-
Before deploying the agent against a new tenant, complete the following steps
98-
in the Conjur tenant:
99-
100-
1. **Enable the authn-jwt authenticator** with `audience=conjur` and
101-
`token-app-property=sub`.
102-
103-
```yaml
104-
# conjur-authn-jwt-policy.yml
105-
- !policy
106-
id: conjur/authn-jwt/disco-agent
107-
body:
108-
- !webservice
109-
110-
- !variable jwks-uri
111-
- !variable token-app-property
112-
- !variable issuer
113-
- !variable audience
114-
115-
- !group hosts
116-
- !permit
117-
role: !group hosts
118-
privilege: [ read, authenticate ]
119-
resource: !webservice
120-
```
121-
122-
2. **Set the authenticator variables** (values shown as examples):
123-
124-
```sh
125-
conjur variable set -i conjur/authn-jwt/disco-agent/token-app-property -v sub
126-
conjur variable set -i conjur/authn-jwt/disco-agent/audience -v conjur
127-
conjur variable set -i conjur/authn-jwt/disco-agent/issuer -v https://kubernetes.default.svc.cluster.local
128-
conjur variable set -i conjur/authn-jwt/disco-agent/jwks-uri -v https://kubernetes.default.svc.cluster.local/openid/v1/jwks
129-
```
130-
131-
3. **Pre-create a Conjur host** for the agent ServiceAccount. The `id` must
132-
match the Kubernetes ServiceAccount's `sub` claim
133-
(`system:serviceaccount:<namespace>:<sa-name>`):
134-
135-
```yaml
136-
# conjur-agent-host-policy.yml
137-
- !host
138-
id: system:serviceaccount/cyberark/disco-agent
139-
annotations:
140-
authn-jwt/disco-agent/sub: system:serviceaccount/cyberark/disco-agent
141-
```
142-
143-
4. **Add the host to the `data/disco/snapshot-uploaders` group** so the
144-
authorizer grants it upload access:
145-
146-
```yaml
147-
- !grant
148-
role: !group data/disco/snapshot-uploaders
149-
member: !host system:serviceaccount/cyberark/disco-agent
150-
```
151-
152-
5. **Add the host to the authn-jwt authenticator's hosts group**:
153-
154-
```yaml
155-
- !grant
156-
role: !group conjur/authn-jwt/disco-agent/hosts
157-
member: !host system:serviceaccount/cyberark/disco-agent
158-
```
97+
Onboarding uses the Conjur Cloud **v2 REST API** (JSON objects), not v1 policy
98+
files — run `hack/onboard-disco-agent.sh` against the target tenant before
99+
deploying the agent. The script performs all of it with the **customer's own**
100+
Conjur Cloud admin token; DisCo holds no Conjur identity of its own:
101+
102+
```sh
103+
CONJUR_TOKEN="$(cat admin-token.txt)" \
104+
hack/onboard-disco-agent.sh \
105+
--ark-subdomain <tenant-subdomain> \
106+
--issuer https://kubernetes.default.svc.cluster.local
107+
```
108+
109+
Tenant-level (once per tenant per provider type): creates the `data/disco`
110+
branch, a per-type uploader group, and a value-less "probe" secret that only
111+
the uploader group can `read`+`execute` — that probe is the entire
112+
authorization decision at request time (no policy `!permit`/`!grant` needed).
113+
114+
Per-cluster (each onboard): creates `data/<provider-type>/<cluster-UUID>` and
115+
its `workloads` sub-branch, an `authn-jwt` authenticator scoped to the
116+
cluster's real OIDC issuer + JWKS, registers the agent's workload under that
117+
branch, and grants it both the authenticator's `apps` group (to authenticate)
118+
and the uploader group (to pass the probe, i.e. to authorize).
119+
120+
`CLUSTER_UUID` (the `kube-system` namespace UID), the OIDC issuer, and the JWKS
121+
are auto-derived from the current `kubectl` context if not passed explicitly.
122+
See the script's header comment for the full `--dry-run`/`--skip-deploy` flag
123+
reference.
159124

160125
### Deploy the agent
161126

@@ -186,7 +151,7 @@ kubectl logs deployments/disco-agent --namespace "${NAMESPACE}" --follow
186151
| Symptom | Likely cause | Fix |
187152
|---|---|---|
188153
| Agent logs `401 Unauthorized` from Conjur | ServiceAccount token `audience` does not match the authenticator's configured `audience` value, or the authn-jwt authenticator is not enabled for the account | Confirm `audience=conjur` in both the projected volume (chart default) and the Conjur `conjur/authn-jwt/<serviceId>/audience` variable; ensure the authenticator is enabled (`CONJUR_AUTHENTICATORS` includes `authn-jwt/<serviceId>`) |
189-
| Agent logs `403 Forbidden` from the upload API | The agent's Conjur host is not a member of `data/disco/snapshot-uploaders` | Add the host to the group per step 4 of the onboarding runbook above |
154+
| Agent logs `403 Forbidden` from the upload API | The agent's workload is not a member of `data/disco/<type>-uploaders` (fails the authz probe) | Re-run `hack/onboard-disco-agent.sh` for this cluster — it's idempotent |
190155
| Agent logs `500` / no upload attempt | Conjur is unreachable or returned an unexpected error | Check network policy / DNS; inspect Conjur audit logs for the host identity |
191156

192157
## Values

deploy/charts/disco-agent/templates/configmap.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ data:
1010
cluster_name: {{ .Values.config.clusterName | quote }}
1111
cluster_description: {{ .Values.config.clusterDescription | quote }}
1212
period: {{ .Values.config.period | quote }}
13+
cyberark:
14+
service_id: {{ .Values.config.cyberark.serviceId | quote }}
15+
account: {{ .Values.config.cyberark.account | quote }}
16+
jwt_source: {{ .Values.config.cyberark.jwtSource | quote }}
17+
jwt_file_path: {{ .Values.config.cyberark.jwtFilePath | quote }}
1318
{{- with .Values.config.excludeAnnotationKeysRegex }}
1419
exclude-annotation-keys-regex:
1520
{{- . | toYaml | nindent 6 }}

deploy/charts/disco-agent/values.schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@
188188
},
189189
"helm-values.config.cyberark.serviceId": {
190190
"default": "",
191-
"description": "The Conjur authn-jwt authenticator service ID configured for this tenant. Example: conjur/authn-jwt/disco-agent",
191+
"description": "The Conjur authn-jwt authenticator service ID configured for this tenant. Bare segment, not the policy path. Example: disco-agent",
192192
"type": "string"
193193
},
194194
"helm-values.config.excludeAnnotationKeysRegex": {

hack/onboard-disco-agent.sh

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,20 @@
1515
# T3 POST /secrets/static -> data/disco/<type>-uploaders-probe
1616
# (NO value; group granted ["read","execute"] — the authz probe object)
1717
# Per-cluster (each onboard)
18-
# C1 POST /branches -> data/<cluster-UUID>
19-
# C2 POST /branches -> data/<cluster-UUID>/workloads
20-
# C3 POST /authenticators -> authn-jwt/<cluster-UUID> (enabled)
21-
# C4 POST /workloads/ -> workload under .../workloads
22-
# C5 POST /groups/<apps>/members -> workload (authenticate grant)
23-
# C6 POST /groups/<uploaders>/members -> workload (authorize grant, Layer 1)
18+
# C1 POST /branches -> data/<type>
19+
# C2 POST /branches -> data/<type>/<cluster-UUID>
20+
# C3 POST /branches -> data/<type>/<cluster-UUID>/workloads
21+
# C4 POST /authenticators -> authn-jwt/<cluster-UUID> (enabled)
22+
# C5 POST /workloads/ -> workload under .../workloads
23+
# C6 POST /groups/<apps>/members -> workload (authenticate grant)
24+
# C7 POST /groups/<uploaders>/members -> workload (authorize grant, Layer 1)
25+
#
26+
# Workload branch is data/<type>/<cluster-UUID>/workloads/<sub> so the agent's
27+
# whoami identity is host/data/<type>/<cluster-UUID>/workloads/<sub> — the
28+
# provider-neutral (provider_type, provider_id) shape the authorizer's
29+
# agent_provider_from_identity() parses (Layer 2: route-type match + FIS
30+
# cluster-forgery check). A bare data/<cluster-UUID>/workloads branch (missing
31+
# the <type> segment) produces an identity the authorizer's parser rejects.
2432
# Then (unless --skip-deploy): create the agent-credentials Secret + helm install.
2533
#
2634
# Requirements: curl, jq, kubectl, helm. A Conjur Cloud admin access token
@@ -139,7 +147,8 @@ if [[ -z "$CONJUR_BASE_URL" ]]; then
139147
fi
140148

141149
SA_SUB="system:serviceaccount:${SA_NAMESPACE}:${SA_NAME}" # real k8s sub claim
142-
CLUSTER_BRANCH="data/${CLUSTER_UUID}"
150+
TYPE_BRANCH="data/${PROVIDER_TYPE}"
151+
CLUSTER_BRANCH="${TYPE_BRANCH}/${CLUSTER_UUID}"
143152
WORKLOAD_BRANCH="${CLUSTER_BRANCH}/workloads"
144153
UPLOADERS_GROUP="data/disco/${PROVIDER_TYPE}-uploaders"
145154
PROBE_SECRET="${UPLOADERS_GROUP}-probe"
@@ -222,13 +231,19 @@ echo
222231
# ---------------------------------------------------------------------------
223232
echo "--- Per-cluster (${CLUSTER_UUID}) ---"
224233

225-
# C1/C2: cluster branch + workloads sub-branch
226-
api POST /branches "{\"name\":\"${CLUSTER_UUID}\",\"branch\":\"data\",\"owner\":{\"kind\":\"group\",\"id\":\"Conjur_Cloud_Admins\"}}" >/dev/null
227-
echo " C1 branch ${CLUSTER_BRANCH}"
234+
# C1/C2/C3: provider-type branch + cluster branch + workloads sub-branch.
235+
# The type segment is required — the authorizer's agent_provider_from_identity()
236+
# parses whoami identities as host/data/<type>/<cluster-UUID>/workloads/<sub>;
237+
# without it, the identity is structurally rejected and Layer-2 (route-type
238+
# match + FIS cluster-forgery check) silently never resolves for this agent.
239+
api POST /branches "{\"name\":\"${PROVIDER_TYPE}\",\"branch\":\"data\",\"owner\":{\"kind\":\"group\",\"id\":\"Conjur_Cloud_Admins\"}}" >/dev/null
240+
echo " C1 branch ${TYPE_BRANCH}"
241+
api POST /branches "{\"name\":\"${CLUSTER_UUID}\",\"branch\":\"${TYPE_BRANCH}\",\"owner\":{\"kind\":\"group\",\"id\":\"Conjur_Cloud_Admins\"}}" >/dev/null
242+
echo " C2 branch ${CLUSTER_BRANCH}"
228243
api POST /branches "{\"name\":\"workloads\",\"branch\":\"${CLUSTER_BRANCH}\",\"owner\":{\"kind\":\"group\",\"id\":\"Conjur_Cloud_Admins\"}}" >/dev/null
229-
echo " C2 branch ${WORKLOAD_BRANCH}"
244+
echo " C3 branch ${WORKLOAD_BRANCH}"
230245

231-
# C3: authn-jwt authenticator named after the cluster-UUID. Trust root is the
246+
# C4: authn-jwt authenticator named after the cluster-UUID. Trust root is the
232247
# REAL cluster OIDC (issuer + jwks/public-keys), never a snapshot.
233248
if [[ -n "$PUBLIC_KEYS" ]]; then
234249
KEYS_FIELD="\"public_keys\":{\"type\":\"jwks\",\"value\":${PUBLIC_KEYS}}"
@@ -241,30 +256,30 @@ api POST /authenticators "$(cat <<JSON
241256
"identity":{"token_app_property":"sub","identity_path":"${WORKLOAD_BRANCH}"}}}
242257
JSON
243258
)" >/dev/null
244-
echo " C3 authenticator authn-jwt/${CLUSTER_UUID} (enabled)"
259+
echo " C4 authenticator authn-jwt/${CLUSTER_UUID} (enabled)"
245260

246-
# C4: the agent workload, under the cluster's workloads sub-branch.
261+
# C5: the agent workload, under the cluster's workloads sub-branch.
247262
api POST /workloads/ "$(cat <<JSON
248263
{"name":"${SA_SUB}","branch":"${WORKLOAD_BRANCH}","type":"other",
249264
"authn_descriptors":[{"type":"jwt","service_id":"${CLUSTER_UUID}","data":{"sub":"${SA_SUB}"}}]}
250265
JSON
251266
)" >/dev/null
252-
echo " C4 workload ${WORKLOAD_BRANCH}/${SA_SUB}"
267+
echo " C5 workload ${WORKLOAD_BRANCH}/${SA_SUB}"
253268

254269
WORKLOAD_ID="${WORKLOAD_BRANCH}/${SA_SUB}"
255270

256-
# C5: authenticate grant — add the workload to the authenticator's apps group.
257-
# The apps-group identifier is created by C3. Confirm its exact id against the
271+
# C6: authenticate grant — add the workload to the authenticator's apps group.
272+
# The apps-group identifier is created by C4. Confirm its exact id against the
258273
# create-authenticator response for your tenant; convention below.
259274
APPS_GROUP="conjur/authn-jwt/${CLUSTER_UUID}/apps"
260275
api POST "/groups/$(urlencode "$APPS_GROUP")/members" \
261276
"{\"id\":\"${WORKLOAD_ID}\",\"kind\":\"workload\"}" >/dev/null
262-
echo " C5 authenticate grant: ${WORKLOAD_ID} -> ${APPS_GROUP}"
277+
echo " C6 authenticate grant: ${WORKLOAD_ID} -> ${APPS_GROUP}"
263278

264-
# C6: authorize grant (Layer 1) — add the workload to the per-type uploader group.
279+
# C7: authorize grant (Layer 1) — add the workload to the per-type uploader group.
265280
api POST "/groups/$(urlencode "$UPLOADERS_GROUP")/members" \
266281
"{\"id\":\"${WORKLOAD_ID}\",\"kind\":\"workload\"}" >/dev/null
267-
echo " C6 authorize grant: ${WORKLOAD_ID} -> ${UPLOADERS_GROUP}"
282+
echo " C7 authorize grant: ${WORKLOAD_ID} -> ${UPLOADERS_GROUP}"
268283

269284
echo
270285
echo "=== Conjur onboarding complete for cluster ${CLUSTER_UUID} ==="

pkg/agent/config_test.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,25 +1330,30 @@ func TestConfig_CyberArk_Validation(t *testing.T) {
13301330
t.Setenv("ARK_SUBDOMAIN", "tlspk")
13311331
}
13321332

1333-
t.Run("empty service_id produces an error", func(t *testing.T) {
1333+
// service_id is no longer required at config-validation time: the agent
1334+
// also supports the legacy username/password method (ARK_USERNAME/ARK_SECRET,
1335+
// set via env, not config), and cyberark.selectAuthenticator fails closed at
1336+
// runtime (ErrNoAuthMethod) if neither method ends up configured. See the
1337+
// comment on this validation block in config.go.
1338+
t.Run("empty service_id is valid at config time", func(t *testing.T) {
13341339
setEnv(t)
1335-
_, _, err := ValidateAndCombineConfig(discardLogs(),
1340+
_, combined, err := ValidateAndCombineConfig(discardLogs(),
13361341
withConfig(testutil.Undent(`
13371342
cyberark:
13381343
service_id: ""
13391344
`)),
13401345
withCmdLineFlags("--period", "1m", "--machine-hub"))
1341-
require.Error(t, err)
1342-
assert.Contains(t, err.Error(), "cyberark.service_id is required in MachineHub mode")
1346+
require.NoError(t, err)
1347+
assert.Equal(t, "", combined.CyberArk.ServiceID)
13431348
})
13441349

1345-
t.Run("missing cyberark block produces a service_id error", func(t *testing.T) {
1350+
t.Run("missing cyberark block is valid at config time", func(t *testing.T) {
13461351
setEnv(t)
1347-
_, _, err := ValidateAndCombineConfig(discardLogs(),
1352+
_, combined, err := ValidateAndCombineConfig(discardLogs(),
13481353
withConfig(""),
13491354
withCmdLineFlags("--period", "1m", "--machine-hub"))
1350-
require.Error(t, err)
1351-
assert.Contains(t, err.Error(), "cyberark.service_id is required in MachineHub mode")
1355+
require.NoError(t, err)
1356+
assert.Equal(t, "", combined.CyberArk.ServiceID)
13521357
})
13531358

13541359
t.Run("jwt_source spiffe is rejected", func(t *testing.T) {

0 commit comments

Comments
 (0)