search

open-feature / open-feature-operator

A Kubernetes feature flag operator
https://openfeature.dev
Apache License 2.0
182 stars 35 forks source link

fix: create index for pod annotation path for allowkubernetessync annotation instead of deployment #582

Closed odubajDT closed 10 months ago

odubajDT commented 10 months ago

Fixes: #579

codecov[bot] commented 10 months ago

Codecov Report

Merging #582 (aa62297) into main (70fb5d9) will not change coverage. The diff coverage is n/a.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #582 +/- ## ======================================= Coverage 87.98% 87.98% ======================================= Files 10 10 Lines 932 932 ======================================= Hits 820 820 Misses 89 89 Partials 23 23 ``` | [Flag](https://app.codecov.io/gh/open-feature/open-feature-operator/pull/582/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) | Coverage Δ | | |---|---|---| | [unit-tests](https://app.codecov.io/gh/open-feature/open-feature-operator/pull/582/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) | `87.98% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature#carryforward-flags-in-the-pull-request-comment) to find out more.
toddbaert commented 10 months ago

Can we add some kind of test for this? e2e or otherwise?

Another question I have is that wrote response {"webhook": "/mutate-v1-pod", "code": 200, "reason": "OpenFeature is disabled", "UID": "8f021e26-6d9b-47f2-90f2-588236ec388d", "allowed": true} (meaning the openfeature/enabled annotation isn't found) is also reported in the issue. Is that related or is that a totally different issue (I think it's different)? Note my comment here.

odubajDT commented 9 months ago

Can we add some kind of test for this? e2e or otherwise?

Another question I have is that wrote response {"webhook": "/mutate-v1-pod", "code": 200, "reason": "OpenFeature is disabled", "UID": "8f021e26-6d9b-47f2-90f2-588236ec388d", "allowed": true} (meaning the openfeature/enabled annotation isn't found) is also reported in the issue. Is that related or is that a totally different issue (I think it's different)? Note my comment here.

Hey, the problem comes from the webhook checking if the examinated pod has the proper annotation openfeature.dev/enabled: "true". According to Adam's manifests, this should be ok. Also if the OF is not enabled, we wouldn't receive the problems with backfilling permissions -> we won't be able to reach this part of code at all.

Maybe Argo was creating a Pod in a completely different namespace at that time (completely unrelated to this Deployment)? Here it would make sense that the podMutator kicks in, checks if this Pod is annotated, finds out it's not, writes out the message and lets the pod to be bind to a node without any mutation? I think here we need to have more info about what was happening in the system generally...