Fixed: Allow to disable owner references for cross namespace access with builder and function namespace #3024
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…deployment. Use this env var to decide adding ownerReferences to K8s resources created by fission CRD. Signed-off-by: Md Soharab Ansari <soharab.ansari@infracloud.io>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3024 +/- ##
==========================================
+ Coverage 44.33% 44.44% +0.10%
==========================================
Files 236 236
Lines 24659 24702 +43
==========================================
+ Hits 10933 10979 +46
+ Misses 12320 12318 -2
+ Partials 1406 1405 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -39,6 +39,10 @@ import ( | |||
| "github.com/fission/fission/pkg/utils/uuid" | |||
| ) | |||
|
|
|||
| const ( | |||
| ENV_DISABLE_OWNER_REFERENCES string = "DISABLE_OWNER_REFERENCES" | |||
There was a problem hiding this comment.
Please add a method in utils
func IsOwnerReferencesEnabled() bool {
return os.Getenv(utils.ENV_DISABLE_OWNER_REFERENCES) != "false"
// use parse bool or something more sophisticated here so that we can identify
// False, FASLE, false etc as well. As this directly gets input from chart.
}
Once above done.
- We should add enable_owner_references flags at struct level whereever it used. Just call util.IsOwnerReferencesEnabled function to get global value.
- Pass flag for functions which are not methods.
- Idea is to not read env variable every time only at struct creation time and every component has its own control.
Signed-off-by: Md Soharab Ansari <soharab.ansari@infracloud.io>
Signed-off-by: Md Soharab Ansari <soharab.ansari@infracloud.io>
| ## If set to true, the K8s resources created by Fission will not have OwnerReference set. | ||
| ## Set to false if you want to add OwnerReference to K8s resources created by Fission. | ||
| ## | ||
| ## Set to true if you are using cross namespace meaning `builderNamespace` and `functionNamespace` are set. |
There was a problem hiding this comment.
Please add note about deprecation.
This flag is temporary addition and would be removed in future fission releases.
Also add deprecation warning in builderNamespace and functionNamespace params.
This can be a separate PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
DISABLE_OWNER_REFERENCESenv variable to executor and buildermgr which would befalseby default. If user is setting cross namespacebuilderNamespaceandfunctionNamespaceaccess then they would need to set it true.disableOwnerReferenceto true via helm chart.Which issue(s) this PR fixes:
Fixes #3023
Testing
Checklist: