cybozu-go / accurate

Kubernetes controller for multi-tenancy. It propagates resources between namespaces accurately and allows tenant users to create/delete sub-namespaces.
https://cybozu-go.github.io/accurate/
Apache License 2.0
38 stars 5 forks source link

Accurate can be erroneously set-up to propagate its own labels/annotations #48

Closed Hsn723 closed 2 years ago

Hsn723 commented 2 years ago

Describe the bug Admitedly, I did not yet try to reproduce the issue as I am not sure if this is intended behavior or not, but currently when configuring Accurate's labelKeys/annotationKeys, no check is performed as to whether Accurate's own labels and annotations have been specified.

For instance, config.Validate only checks if the labels/annotations are syntatically valid: https://github.com/cybozu-go/accurate/blob/6773c56d67c2a180c93aadb30b096d4d9e587232/pkg/config/types.go#L37-L43

As a result, an administrator could set up Accurate's config.yaml to propagate Accurate's own labels and annotations and it would happily oblige.

Accurate controller's propagateMeta will indeed propagate all configured labels/annotations: https://github.com/cybozu-go/accurate/blob/6773c56d67c2a180c93aadb30b096d4d9e587232/controllers/namespace_controller.go#L90-L94

Granted, this is purely user misconfiguration, though I think that adding a check for the mistaken specification of labels/annotations in the constants.MetaPrefix namespace (or specific labels/annotations that should definitely not be configured to propagate?) would be more user-friendly.

Environments

To Reproduce Steps that should reproduce the behavior:

  1. Add the following to config.yaml
    labelKeys:
    - accurate.cybozu.com/type
  2. Deploy Accurate
  3. Add a SubNamespace
  4. It should have inherited the accurate.cybozu.com/type=root from its parent namespace

Expected behavior I am not exactly sure if this is expected behavior, but it does not seem like it is originally intended to happen.

Additional context

ymmt2005 commented 2 years ago

I agree with your opinion. Will you create a PR to fix this?

Hsn723 commented 2 years ago

Thanks for the feedback. I'll give it a go.