weave-gitops icon indicating copy to clipboard operation
weave-gitops copied to clipboard

Feature flags implementation is problematic

Open jpellizzari opened this issue 2 years ago • 3 comments

With telemetry turned on and no flags.ACCOUNT_ID present, the UI explodes.

This line can resolve to undefined or some other value that does not have a .match method:

https://github.com/weaveworks/weave-gitops/blob/661f3f27f1e97ccf0aa94b8c8658dea7a2f5e883/ui/components/Pendo.tsx#L89

In general, we should not be passing data as a feature flag. These flags should be true/false to enable or disable parts of the UI. Our PENDO_ACCOUNT_ID should be some sort of env var or secret.

In addition, it appears we are relying on some magic strings on the backend to generate the token:

https://github.com/weaveworks/weave-gitops/blob/661f3f27f1e97ccf0aa94b8c8658dea7a2f5e883/pkg/telemetry/telemetry.go#L28

There appears to be some sort of encryption/hashing happening here, but then that value would be decrypted and sent in plain text to the Pendo API. I would have to consult the Pendo docs to understand if this is a suggested approach or not.

It is also a bit odd that we are using the Namespace.UID as an input to the hasing.

jpellizzari avatar Mar 29 '23 18:03 jpellizzari

Here is the initial Pendo integration PR which might provide more context: https://github.com/weaveworks/weave-gitops/pull/2652

opudrovs avatar Mar 29 '23 18:03 opudrovs

This completely abuses the feature flag system

I'm glad the author and I agree :). Some tech debt we need to pay down.

jpellizzari avatar Mar 29 '23 19:03 jpellizzari

@jpellizzari Can you review this as part of "common UI" topics?

lasomethingsomething avatar Jul 24 '23 15:07 lasomethingsomething