troubleshoot icon indicating copy to clipboard operation
troubleshoot copied to clipboard

Add support to CLI to search for all configMaps that contain Troubleshoot redactors

Open xavpaice opened this issue 3 years ago • 5 comments

Request

Ref design #663, and task #699

Currently Troubleshoot can use configMaps for Redactor specs, and when called within KOTS we can see multiple redactors added to the defaults built in. With #683 we add the ability to list multiple specs on the CLI, but not a wildcard, directory, or search pattern.

This request is to implement a search function whereby any configMaps that can be found with a matching label (default to troubleshoot.io/kind=redactor-spec), will be collected and used as if they had been specified on the CLI. This search should be made in the namespace defined in the same way as the kubectl client.

In scope

  • add a switch to the CLI, which triggers Troubleshoot to search for configMaps
    • in the namespace specified with -n or --namespace
    • all namespaces if no namespace is specified
    • for any configMap with the label troubleshoot.io/kind=redactor-spec unless the -l switch is provided to override that
  • Include each configMap redactor found as if it had been specified on the CLI (i.e. use them all)
  • Test coverage
  • Documentation

NOTE: the -l switch is already used in #699 and we should discuss if we want to share the switch or make a new one. Same for -n though it's more likely this should be shared.

Out of scope

  • KOTS changes to make use of this addition

NOTE

The package "k8s.io/cli-runtime/pkg/genericclioptions" may be helpful with the kubectl options, see this example.

This task may be affected by work on #694, which uses the URI specified in a spec only on the specs provided on the CLI. To be clear, the specs found in secrets with this task need to be considered as specs provided on the CLI, therefore the URI will need to be read and used.

Also: Note that KOTS uses the kots.io/kind=supportbundlelabel here to describe collected support bundles.

Kots does store a Troubleshoot spec, with the name kotsadm-$APPSLUG-supportbundle, however it does not currently have a label which can be used to identify it. Also redactors are stored in configMaps kotsadm-redact, kotsadm-redact-spec and kotsadm-appslug-redact-spec amonst possibly other places. These are not labeled other than with default kots labels.

Definition of done

  • [ ] CLI switch enabling Troubleshoot to search for and use all configMaps with a label troubleshoot.io/kind=redactor-spec (default), or a CLI specified label
  • [ ] CLI supports kubectl's -l, --selector='' switch to override the label searched for
  • [ ] CLI supports kubectl's -A, --all-namespaces, -n, --namespace switches to override namespace
  • [ ] Unit tests for the new implementation
  • [ ] Documentation updated to show usage of this new feature in https://troubleshoot.sh/docs

xavpaice avatar Sep 15 '22 06:09 xavpaice

@xavpaice I know this makes sense in the context of how things work today, but I wonder if we should just make redactors available in support bundle specs and then allow them to be discovered and merged with the below? We could then just search for supportbundle/redactors in the same spec across both configmaps and secrets as opposed to searching for them in different objects.

https://github.com/replicatedhq/troubleshoot/pull/683 https://github.com/replicatedhq/troubleshoot/pull/709

diamonwiggins avatar Sep 15 '22 21:09 diamonwiggins

So if I'm following, what you're suggesting is don't add another label type. Just combine redactors into the support-bundle-spec and allow the search to pull everything from both configMaps and Secrets so the user can put (eventually) Collectors, Analyzers, and Redactors in Secrets or ConfigMaps and it all just gets combined together and used in a single spec since they all have unique keys as well?

If that's what you're suggesting, that sounds like a clean path forward to me with less edge cases to confuse users. If that's roughly the same (or possibly less?) work to do and doesn't break software that imports troubleshoot then this seems like a good path forward. I could see if this breaks KOTS for example we might want to first implement just the label as suggested above and address moving things in the specs after we have a stable API to refer people to.

chris-sanders avatar Sep 16 '22 15:09 chris-sanders

I agree that Redactors could be moved into SupportBundle so we can avoid all the multidoc, secrets/configMap separation. However, that's not how it is today and we do need a good way to migrate to that pattern if we choose to do it.

It does seem reasonable that rather than adding a second label for redactors, we could have the same label for supportBundle specs and Redactors. That way, the future picture is a little easier.

Redactors do have the possibility to contain private info though - the replace redactor could easily contain a password which someone doesn't want to make public.

xavpaice avatar Sep 16 '22 19:09 xavpaice

I'm wondering now if we really need the ability to override the label selector or if we should just hardcode it to a value we expect? Do we really want three different "selector" CLI flags for each kind? SupportBundle, Preflight, and Redactor? I think it would also be cleaner if we could reuse the load-cluster-specs flag for everything. Were we thinking different flags for each kind as well?

I'm also planning with this change to search for SupportBundles/Redactors in both ConfigMaps and Secrets if no objections. Getting started with all of the above suggestions in mind, and can continue the discussion here about the ideal end state.

diamonwiggins avatar Oct 03 '22 17:10 diamonwiggins

I think the label switch is largely to take into account the needs of non-Replicated folks that might be using a different label - it's lower priority but good to have if it's 'cheap'.

Regards the search for both ConfigMaps and Secrets, I'm struggling to understand exactly why Redactors are in ConfigMaps and not Secrets - and would be happy for the search to cover both, if that's a more straightforward change.

xavpaice avatar Oct 03 '22 20:10 xavpaice

I'm going to close this without the docs update, as we currently don't have any cli docs.

xavpaice avatar Nov 15 '22 06:11 xavpaice