Check for the permissions instead of using a CLI flag #2787
Conversation
…#2588 Signed-off-by: Israel Blancas <iblancasa@gmail.com>
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
Sure. I'm implementing them. |
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
…or into feature/2588
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
…or into feature/2588
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
.chloggen/replace-create-rbac-permissions-by-checking-the-sa-permissions.yaml
Outdated
Show resolved
Hide resolved
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
…or into feature/2588
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
…or into feature/2588
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
There was a problem hiding this comment.
where does these files actually get used? How would a user know to add these into their rbac?
There was a problem hiding this comment.
I think these can go into the /tests/e2e-automatic-rbac directory. My request to use patches instead of overriding the whole default ClusterRole aimed to keep e2e test setup consistent. It's good if our e2e tests use the kustomize manifests roughly the way we expect users to, but these files don't literally need to be user-facing in any way.
Does that make sense?
There was a problem hiding this comment.
yeah that makes sense to me! As it stands rn these feel very out of place here if we're not really expecting users to apply them
There was a problem hiding this comment.
I put the files under config/rbac because this error is thrown by kustomize:
Error: accumulating resources: accumulation err='accumulating resources from '../rbac': '/home/iblancasa/projects/opentelemetry-operator/config/rbac' must resolve to a file': recursed accumulation of path '/home/iblancasa/projects/opentelemetry-operator/config/rbac': trouble configuring builtin PatchTransformer with config: `
path: ../../tests/e2e-automatic-rbac/extra-permissions-operator/namespaces.yaml
target:
kind: ClusterRole
name: manager-role
`: security; file '/home/iblancasa/projects/opentelemetry-operator/tests/e2e-automatic-rbac/extra-permissions-operator/namespaces.yaml' is not in or below '/home/iblancasa/projects/opentelemetry-operator/config/rbac'
error: no objects passed to applyI added something to workaround this. Let me know what you think about :)
| - apply: | ||
| file: 02-install.yaml | ||
| - assert: | ||
| file: 02-assert.yaml |
There was a problem hiding this comment.
(can be done in a follow up) both this test and the k8sattr test would benefit from us actually verifying that the collector is doing the work as expected and not just failing. we could probably achieve this by using the debug exporter and send some telemetry through the system to verify it's logged with what we want.
There was a problem hiding this comment.
I have been thinking about that since I created the test. Is it ok to use the contrib image for that? I can do it in a different PR yes.
There was a problem hiding this comment.
does the k8s distro have all the things we need?
There was a problem hiding this comment.
Oh, actually it should. Didn't remember you added/are adding that. Thanks!
|
Just a few more Qs, once those are answered I think we're ✅ |
|
The PR needs to be rebased |
…or into feature/2588
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
|
Are any other changes required? |
…or into feature/2588
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
…or into feature/2588
| apiVersion: v1 | ||
| kind: Namespace | ||
| metadata: | ||
| name: chainsaw-k8sattributes |
There was a problem hiding this comment.
I think we can drop this and just use $(NAMESPACE) wherever we need it in tests.
…or into feature/2588
…or into feature/2588
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
Closes #2588
Closes #2833
Closes #2862