jsonnet/prometheus: configure permissions #74
Conversation
openshift-ci-robot
added
the
size/M
manifests/prometheus/list.yaml
Outdated
| "namespace": "telemeter-production"}' | ||
| - name: OPENSHIFT_DELEGATE_URLS | ||
| value: '{"/": {"resource": "namespaces", "verb": "get", "resourceName": "telemeter-production", | ||
| "namespace": "telemeter-production"}}' |
There was a problem hiding this comment.
Does it really need to be that specifically configurable? Why not just use the namespace and template it into the SubjectAccessReview?
There was a problem hiding this comment.
this was just "on suspicion" to have a broad configurability. We can reduce it to namespace only 👍
| @@ -20,6 +20,8 @@ local k = import 'ksonnet/ksonnet.beta.3/k.libsonnet'; | |||
| rules: { groups: [] }, | |||
| htpasswdAuth: '', | |||
| sessionSecret: '', | |||
| sar: '{"resource": "namespaces", "verb": "get", "resourceName": "telemeter-production", "namespace": "telemeter-production"}', | |||
There was a problem hiding this comment.
this seems like a bit too much configurability here. I think we can optimize for our actual expected use case :), since we know how it is intended to be deployed. Anyone wild enough to use and want more configurability power this can import the jsonnet and use additional mixins to modify the generated manifests
There was a problem hiding this comment.
ack, see #74 (comment)
| @@ -20,6 +20,8 @@ local k = import 'ksonnet/ksonnet.beta.3/k.libsonnet'; | |||
| rules: { groups: [] }, | |||
| htpasswdAuth: '', | |||
| sessionSecret: '', | |||
| sar: '{"resource": "namespaces", "verb": "get", "resourceName": "telemeter-production", "namespace": "telemeter-production"}', | |||
There was a problem hiding this comment.
currently $._config.namespace defaults to telemeter and has one config variable, but the SAR uses telemeter-production as the target namespace. Rather than having to configure both $._config.namespace as well as the SAR so that the namespaces match, the SAR should inherit the namespace to which the object is deployed.
There was a problem hiding this comment.
ah, that's indeed a good idea
This enables permissions (subject access review as well as delegate URLs) to be configured using saas hurder template variables.
s-urbaniak
force-pushed
the
prom-perms
branch
from
cf822cb
to
38c9344
Compare
|
Looks good. Giving @squat the last call. |
|
much better |
|
/lgtm |
This enables permissions (subject access review as well as delegate URLs)
to be configured using saas hurder template variables.
cc @squat