clean up operator initialization code #2901
Merged
Conversation
This file contains 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
flyth
force-pushed
the
michael/oci/fix-validate-settings
branch
from
4d8fa77
to
256ca84
Compare
flyth
requested review from
blanquicet,
alban and
mauriciovasquezbernal
as code owners
This was referenced May 27, 2024
mauriciovasquezbernal
approved these changes
May 28, 2024
There was a problem hiding this comment.
LGTM. Thanks for handling it.
gadgettracermgr should also expose those params exposed by the gadget-service, this is however currently not handled in this PR (but maybe should also be added; using env variables on deployment maybe?)
I think it can be handled once we have #2831 (cc @mqasimsarfraz)
I noticed that the order of the commits breaks the compilation for some of them, can it be fixed?
| @@ -89,6 +93,12 @@ func NewRunCommand(rootCmd *cobra.Command, runtime runtime.Runtime, hiddenColumn | |||
|
|
|||
| ops := make([]operators.DataOperator, 0) | |||
| for _, op := range operators.GetDataOperators() { | |||
There was a problem hiding this comment.
Don't we have global parameters for layer operators? I suppose we'll need to add them later on.
There was a problem hiding this comment.
Not yet, but yeah, we should add them sooner or later.
flyth
force-pushed
the
michael/oci/fix-validate-settings
branch
2 times, most recently
from
3c9f4e7
to
87eb57a
Compare
This change collects available operators when starting the service. Global operator params can be retrieved through `GetOperatorMap()`. Signed-off-by: Michael Friese <mfriese@microsoft.com>
This allows setting the eventBufferLength after creating the service. Will be used in upcoming commits. Signed-off-by: Michael Friese <mfriese@microsoft.com>
Signed-off-by: Michael Friese <mfriese@microsoft.com>
…mage-based gadgets Signed-off-by: Michael Friese <mfriese@microsoft.com>
Operator initialization should be handled by the caller that is also creating the gadget context, not the gadget context itself. This removes the old code and relies on initialization taking place somewhere else. Signed-off-by: Michael Friese <mfriese@microsoft.com>
flyth
force-pushed
the
michael/oci/fix-validate-settings
branch
from
87eb57a
to
6e250d5
Compare
|
Thanks for the review! |
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.
Previously, for image-based gadgets, operators would lazily be initialized by the gadget context. That made it difficult to forward the actual global configuration of operators. Since we didn't really use those in the past, it was not a big issue.
However, now that we're for example using image verification that can be enabled/disabled using parameters, we need to clean this up. Currently, the client decides whether image validation is done by sending the appropriate flag to the server. This is however insecure, as the client shouldn't have authority over that decision. We may later on allow (using another param/setting that can be enabled on the server) clients to override certain behavior, but in general the server should decide about the security features.
This PR moves the initialization to all our entry points for image-based gadgets and exports the parameters to be set there, if applicable.
This means:
Fixes #2891