Add proxyProfile support to qdr pkg#2395
Add proxyProfile support to qdr pkg#2395ajssmith wants to merge 6 commits intoskupperproject:mainfrom
Conversation
| for _, profile := range desired { | ||
| current, ok := actual[profile.Name] | ||
| if !ok { | ||
| if err := agent.CreateProxyProfile(profile); err != nil { |
There was a problem hiding this comment.
Should it just call continue here, if err == nil?
There was a problem hiding this comment.
Yes, makes sense. Will adjust.
| } | ||
|
|
||
| func UpdateRouterConfig(client kubernetes.Interface, ctxt context.Context, update *qdr.RouterConfig, configmap *corev1.ConfigMap) error { | ||
| return retry.RetryOnConflict(retry.DefaultRetry, func() error { |
There was a problem hiding this comment.
I am wondering if the retry mechanism will actually work or not here.
If a conflict is found, further attempts to update it might result on a new error, unless the latest revision of the ConfigMap is retrieved, i.e.:
| proxyConfig.Port = string(proxySecret.Data["port"]) | ||
| proxyConfig.User = string(proxySecret.Data["username"]) | ||
| } else { | ||
| return nil, stderrors.New("Secret not found for proxy configuration") |
There was a problem hiding this comment.
Maybe include the secret name that was not found and the optional error.
| wantLinks: 1, | ||
| skupperErrorMessage: "NotFound", | ||
| }, | ||
| // { |
There was a problem hiding this comment.
Are these tests disabled on purpose or by accident?
|
|
||
| func updateRouterConfig(client kubernetes.Interface, ctxt context.Context, update *qdr.RouterConfig, configmap *corev1.ConfigMap) error { | ||
|
|
||
| err := update.WriteToConfigMap(configmap) |
There was a problem hiding this comment.
So I understand that the latest revision of the configmap should be loaded here.
|
|
||
| func (s *Sync) handle(key string, secret *corev1.Secret) error { | ||
| if secret == nil { | ||
| return nil |
There was a problem hiding this comment.
If a proxyProfile's secret is removed, it is not being removed from: Sync.configuredProxy.
|
When I create a Link with no proxy configuration, I am seeing an empty proxyProfile set on the router and an error on kube-adaptor, saying ‘missing secret for profile “”’
[
"proxyProfile",
{}
] |
|
Once a proxyProfile is added into the router configuration, it is not being removed. |
| delta.Missing = append(delta.Missing, profileName) | ||
| continue | ||
| } else { | ||
| _, err := s.handleProxyProfile(key, secret) |
There was a problem hiding this comment.
LLM thinks ExpectProxyProfiles may be passing the wrong key into handleProxyProfile. It fetches the secret using namespace + "/" + profileName, but then calls handleProxyProfile(key, secret), where key is the router ConfigMap key. Since handleProxyProfile stores that argument as syncContext.SecretKey, the stored key appears to become the ConfigMap key rather than the proxy Secret key.
This may not break the current path because ExpectProxyProfiles reconstructs the proxy Secret key directly, but it could make recovery or future code that reads context.SecretKey look up the wrong object. Should this pass the secret key instead?
fgiorgetti
left a comment
fgiorgetti
left a comment
There was a problem hiding this comment.
Good stuff @ajssmith ! It is working great now.
| // secrets for services with TLS enabled, and secrets and connectors for links | ||
| // as well as proxy profiles) | ||
| type ConfigSync struct { | ||
| cli internalclient.Clients |
3 participants
Resolves #2391, Resolves #2393