Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add initial tpl validation for plugins #4235

Merged
merged 3 commits into from
Feb 8, 2022

Conversation

antgamdia
Copy link
Contributor

Description of the change

While I was enabling/disabling plugins in my local env, I've spent some time wondering what was wrong in my setup... it turned to be just a typo in the plugin name 馃う

This PR adds some validation to the plugin names we are using, at least, for the mistakes I use to make :P
Feel free to suggest more.

Benefits

No "OSI layer 8"-derived problems, haha

Possible drawbacks

N/A

Applicable issues

N/A

Additional information

    kubeapps: kubeappsapis.enabledPlugins 
        You enter "flux", perhaps you meant "fluxv2"?
    kubeapps: kubeappsapis.enabledPlugins 
        You enter "kapp_controller", perhaps you meant "kapp-controller"?
    kubeapps: kubeappsapis.enabledPlugins 
        If you enable the "fluxv2" plugin, you must also set redis.enabled=true

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
If you enable the "fluxv2" plugin, you must also set redis.enabled=true
{{- end -}}
{{- end -}}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh - I like dealing with the typo, just not sure we should do so here in the chart in an ad-hoc way. Could we not just update the kubeapps-apis binary so that if any plugin fails or is not found, we print out the names of all the plugins available in the plugin dirs? (We already have code to list those, I tihnk). That would deal with everything except the hint to also enable redis, but that's a temporary thing anyway (in that, we should update so that it is enabled if you are using fluxv2).

Anyway, leaving a +1 in case you have a strong reason to prefer it here in the chart.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also realised your validation here will trigger before the chart deploys, which is a bit nicer from a UX POV.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, we are just adding the plugin if it exists, but doing nothing if it does not. I agree the kubeapps-apis bin should deal with this situation; likely listing the available plugin list could make the trick.

However, as you pointed out later, this requires the chart to be deployed and someone to have a look at the logs. So, in practice, I would have faced the same issue (wondering why it isn't working! what have I broken?) until I would have realized it was an issue with a plugin.
So, even if doing these checks in the templating step is kind of dodgy, I still believe it is useful for our clumsy days :P

@antgamdia antgamdia merged commit 62d2f55 into vmware-tanzu:main Feb 8, 2022
@antgamdia antgamdia deleted the helmPluginValidation branch February 8, 2022 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants