-
Notifications
You must be signed in to change notification settings - Fork 199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLI should not use non-public Cilium APIs #1896
Comments
Would it be reasonable to do this autodetection from the helm configuration level or using |
yeah i was thinking we should just look at cilium-config config map 💡 |
FWIW, the PR adding the runtime feature detection (#1022) has some context on why this is needed and why the information cannot easily be gathered from the ConfigMap:
/cc @gandro |
that's a good point. i guess helm values will have the same issue? 💭 |
Yeah, I think we'll have the same issue with helm values. IIRC we don't specify defaults for them (in all cases). |
struggle |
There was also an issue with the default used for the tunneling feature, which tried to use the config-map value if available (da5c0c9). Therefore I tried to document the current guidelines in the code. I mostly copied the descriptions from the commits, bit I thought it makes sense to also have them in code (7c54c5f). |
I agree that relying on the I think one way answer here is to start exposing per-cell configurations in the Cilium API. We already have an API endpoint that exposes a subset the global runtime daemon configuration https://github.com/cilium/cilium/blob/95e25bfb132750e97304dfbe0f5770f4b6debd96/api/v1/openapi.yaml#L74-L85 The |
I wonder if we could put some infrastructure in place so that those configuration API endpoints are somehow tied to feature stability in the code. For instance, in the case from the issue description, the feature is brand new, so could be considered alpha. Maybe for alpha options we don't expose it in the API because it's subject to change. Then we wouldn't trigger this overall issue in the first place because Cilium-CLI wouldn't be relying on an alpha setting having a specific type or format. For other settings, we could graduate the features themselves to beta/GA and somewhere later in that process we start exposing the flags as a public API from Cilium. |
The following code uses a non-public API from https://github.com/cilium/cilium in order to extract configurations out from an individual agent:
cilium-cli/connectivity/check/features.go
Line 274 in e97bddd
There are a few issues with this:
connectivity test failed: unmarshaling cilium runtime config json: json: cannot unmarshal bool into Go struct field DaemonConfig.HubbleRedact of type []string
This code should probably unmarshal the configuration into a more liberal type to account for differences between Cilium versions.
The text was updated successfully, but these errors were encountered: