Repository navigation
Conversation
Lets operator users run FeatureStore pods under an existing ServiceAccount (e.g. one annotated for IRSA or Vault) and attach extra pod labels for log collectors or policy engines, without the Kyverno workarounds that previously reassigned the operator-created ServiceAccount and its RBAC bindings out of band. Signed-off-by: dbbvitor <vitor.diniz@gympass.com>
Add a "Running under a custom ServiceAccount" section to the security guide with an IRSA/Vault example, and note that the operator binds RBAC to the given ServiceAccount but does not create or annotate it. Signed-off-by: dbbvitor <vitor.diniz@gympass.com>
Add ServerConfigs.commandPrefix, prepended to the operator-generated server command, so users can wrap the Feast server with tools such as opentelemetry-instrument or a profiler without overriding the computed command (port, TLS, workers, metrics) themselves. A verbatim command/args override was rejected in favor of this because it makes the operator's flags easy to silently drop as it evolves. Signed-off-by: dbbvitor <vitor.diniz@gympass.com>
setLineageDeployment builds its container Command directly rather than through getContainerCommand, and had no test asserting on it with commandPrefix set. Add one, verifying the prefix is prepended and Args is unaffected. Signed-off-by: dbbvitor <vitor.diniz@gympass.com>
Add a "Wrapping the server command" section to the serving guide with an opentelemetry-instrument example, and note that the wrapper binary must already be in the image and that init containers are not wrapped. Signed-off-by: dbbvitor <vitor.diniz@gympass.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7023 +/- ##
=======================================
Coverage 50.94% 50.94%
=======================================
Files 455 455
Lines 56915 56915
Branches 8290 8290
=======================================
Hits 28994 28994
Misses 25940 25940
Partials 1981 1981
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
A prior merge-conflict resolution kept arbitrary line numbers for already-allowlisted secrets instead of the real ones, which detect-secrets' CI run (unlike a local run) treats as a hard failure when it needs to rewrite the baseline. No entries added or removed. Signed-off-by: dbbvitor <vitor.diniz@gympass.com>
…rator-command-prefix
commandPrefix was missing from the CRD schema's McpServerConfig-nested ServerConfigs block in config/crd/bases/feast.dev_featurestores.yaml and dist/install.yaml -- the manual regeneration during an earlier rebase missed this one of several ServerConfigs embeddings. Re-ran controller-gen + kustomize build to regenerate both consistently. Also refreshes .secrets.baseline line numbers shifted by the above. Signed-off-by: dbbvitor <vitor.diniz@gympass.com>
commandPrefix was also missing from bundle/manifests/feast.dev_featurestores.yaml, a separate generation target (make bundle) from config/crd/bases and dist/install.yaml fixed in the previous commit. Re-ran the full bundle sequence (operator-sdk generate kustomize manifests, kustomize build, operator-sdk generate bundle) so all three stay in sync. Signed-off-by: dbbvitor <vitor.diniz@gympass.com>
This branch has not been deployed
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
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.
What this PR does / why we need it:
Adds
ServerConfigs.CommandPrefix(new field, next to the existingDualStackin the same struct), prepended to the operator-generated server command (online/offline/registry/UI servers and the lineage server), so users can wrap the Feast server with tools such asopentelemetry-instrumentor a profiler. This deliberately diverges from the issue's original ask for a verbatimcommand/argsoverride: a verbatim override would force users to copy every flag the operator computes (port, TLS, workers, metrics,--registry_ttl_sec, ...), and those copies silently rot as the operatorevolves, we hit exactly this with a Kyverno-based command-wrapping workaround we run internally, which silently dropped
--workers/--metricsonce the operator started setting them. With a prefix, the operator keeps owning the argv, confirmed by readinggetContainerCommand(): every computed flag is appended before the finalreturn withCommandPrefix(feastCommand, ...)call, so the prefix can never clobber a flag.Validated end-to-end by wrapping the online server's command with
opentelemetry-instrumentvia this field, in place of a Dockerfile-level binary-rename shim we use for the same purpose today.Which issue(s) this PR fixes:
Part of #6864
Checks
git commit -s)Testing Strategy
Misc