Skip to content

Commit

Permalink
Ensure loaded plugin config is saved to helm server. (#6022)
Browse files Browse the repository at this point in the history
<!--
Before you open the request please review the following guidelines and
tips to help it be more easily integrated:

 - Describe the scope of your change - i.e. what the change does.
 - Describe any known limitations with your change.
- Please run any tests or examples that can exercise your modified code.

 Thank you for contributing!
 -->

### Description of the change

<!-- Describe the scope of your change - i.e. what the change does. -->
In [#5339 we inadvertently
switched](https://github.com/vmware-tanzu/kubeapps/pull/5339/files#diff-da1b1406426f28c45f4b63a5ae7547e9c29747e118bf70df6a4694d575710f94R104)
from an [assignment](https://go.dev/ref/spec#Assignment_statements) to a
[short variable
declaration](https://go.dev/ref/spec#Short_variable_declarations):

```diff
-		pluginConfig, err = common.ParsePluginConfig(pluginConfigPath)
+		pluginConfig, err := common.ParsePluginConfig(pluginConfigPath)
```

which meant that, in context, a *new* `pluginConfig` var was created in
that scope, so that the actual `pluginConfig` var from the outside scope
remained empty.

### Benefits

Plugin config works again for the Helm plugin.

### Possible drawbacks

<!-- Describe any known limitations with your change -->

### Applicable issues

<!-- Enter any applicable Issues here (You can reference an issue using
#) -->

- fixes #6020 

### Additional information

<!-- If there's anything else that's important and relevant to your pull
request, mention that information here.-->
I spent some time trying to write a test for this, but I could either
extract the 5 lines to a separate function and test that or test the
`NewServer` function itself. The latter is very difficult because it
instantiates a bunch of network requests, and the former seems redundant
since we already extract the functionality to the
`common.ParsePluginConfig()` function... it's really just a golang
gotcha that is important to be aware of: `a := foo` is equivalent to
`var a atype \n a = foo` and so is always creating a new variable in
that scope.

Signed-off-by: Michael Nelson <minelson@vmware.com>
  • Loading branch information
absoludity committed Feb 24, 2023
1 parent 7823b23 commit 56bc140
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ func NewServer(configGetter core.KubernetesConfigGetter, globalPackagingCluster
// If no config is provided, we default to the existing values for backwards
// compatibility.
pluginConfig := common.NewDefaultPluginConfig()
var err error
if pluginConfigPath != "" {
pluginConfig, err := common.ParsePluginConfig(pluginConfigPath)
pluginConfig, err = common.ParsePluginConfig(pluginConfigPath)
if err != nil {
log.Fatalf("%s", err)
}
Expand Down

0 comments on commit 56bc140

Please sign in to comment.