flux plugin to use targetNamespace for HelmRelease CRD per #3640 #3662
Conversation
| if err != nil { | ||
| return nil, err | ||
| } | ||
| newRelease, err := resourceIfc.Create(ctx, fluxHelmRelease, metav1.CreateOptions{}) | ||
| if err != nil { | ||
| if errors.IsForbidden(err) || errors.IsUnauthorized(err) { | ||
| // TODO (gfichtenholt) I think in some cases we should be returning codes.PermissionDenied instead, | ||
| // but that has to be done consistently accross all plug-in operations, not just here |
There was a problem hiding this comment.
+1. I'd personally go for correctness over consistency and do the right thing here, returning permission denied. But up to you.
There was a problem hiding this comment.
I think will will evolve into separate issue at some point across all plug-ins. Direct helm is has this problem too in spades. We'll need to sweep through the code to distinguish all the use cases when 401 is applicable vs. 403. Separate issue. I can file one if you like
cmd/kubeapps-apis/server/plugins.go
Outdated
| @@ -111,7 +111,8 @@ func sortPlugins(p []*plugins.Plugin) { | |||
|
|
|||
| // GetConfiguredPlugins returns details for each configured plugin. | |||
| func (s *pluginsServer) GetConfiguredPlugins(ctx context.Context, in *plugins.GetConfiguredPluginsRequest) (*plugins.GetConfiguredPluginsResponse, error) { | |||
| log.Infof("+core GetConfiguredPlugins") | |||
| // this gets logged twice(why?) every 10 seconds and really adds a lot of noise to the logs, so lowering verbosity | |||
There was a problem hiding this comment.
It's currently used for both the liveness and readiness checks for the deployment.
There was a problem hiding this comment.
ok, I will update the comment, but still reduce log message severity. Too much noise
| @@ -312,6 +312,23 @@ func kubeDeleteServiceAccount(t *testing.T, name, namespace string) error { | |||
| return nil | |||
| } | |||
|
|
|||
| func kubeCreateNamespace(t *testing.T, namespace string) error { | |||
There was a problem hiding this comment.
Is this function added as part of some other work? I don't see it being used here at all.
There was a problem hiding this comment.
that just means you didn't look hard enough. Check release_integration_test.go
There was a problem hiding this comment.
Heh - ctrl-f failed because I'd not noticed the integration test changes (collapsed due to size, and I'd flicked past thinking they were proto changes.
Just updating plug-in code per discussion in
#3640 (comment)