allow extending flux runtime ui beyond flux to create gitops runtime

[enekofb]

Closes of #4174

What changed?

• added api endpoint to support for listing runtime objects beyond flux
• added api endpoint to support for listing runtime crds beyond flux
• addded ui components to support generic runtime api endpoint

Why was this change made?

• to extend current runtime experience beyond flux to includes other oss components like policy or tf-controllers

How was this change implemented?

• Duplicated api endpoints
• Duplicated ui components
• Added feature flag to control the behaviour

How did you validate the change?

• UI
• Unit

Release notes

• Runtime UI supports runtime components beyond flux like policy agent or tf-controller. See documentation for more info.

Documentation Changes

• Added via helm chart reference annotations and updated documentation

[enekofb]

next is to refactor logic to:

• add endpoint gitops_runtime behind the feature flag
• add gitops_runtime ui component
• deprecate flux_runtime one

[opudrovs]

Some tests are failing.

Will test the current PR after I am done with testing my enterprise PR (building it now).

[enekofb]

Some tests are failing.

Will test the current PR after I am done with testing my enterprise PR (building it now).

@opudrovs sorry i assigned request too early, ignore it so far ... just reviewing the PR and need to add questions to ask.

[opudrovs]

@enekofb ah got it, np. 👌

[opudrovs]

❔ Will any of new components/hooks/functions be used in enterprise directly? I mean, should anything be added to exports in ui/index.ts?

[opudrovs]

❔ It's not an issue, just a minor question, related to usability (and I am not sure who will use this new UI, so don't know if it's of any importance). Since the routes of both views are slightly different:

/flux_runtime/flux
/runtime/flux

if the user toggles the feature flag and just refreshes the page (while browser stays at the same route), there will be a page not found error displayed:

Is it possible that someone will share a link to a runtime page (in a documentation, for example) and then the flag will be toggled and the users will bump into page not working? Would a redirect or keeping both routes functional help? If it does not matter (or like in "if anyone starts screaming about it, we'll figure it out then" 😅 ), never mind.

[opudrovs]

LGTM, great job! ✨

Tested it in Tilt with the flag on and off, everything is working as expected:

Just added some minor optional questions and suggestions.

[opudrovs]

Nice! ✨

[opudrovs]

It's great this condition was replaced, ? true : false was redundant here.

❔ But now it looks like the condition below on line 84:

{!supportMultipleFlux && deployments[0]?.labels[fluxVersionLabel] && (
  <FluxVersionText color="neutral30" titleHeight={true}>
    This cluster is running Flux version:
    <span>{deployments[0].labels[fluxVersionLabel]}</span>
  </FluxVersionText>
)}

will never be true because supportMultipleFlux is always true? Does it mean that this FluxVersionText component will never get rendered? Or are you going to change this condition later?

And this check on line 71 is not needed too, because now it is always true:

  if (supportMultipleFlux) {
    tabs.unshift({
      name: "Flux Versions",
      path: `${path}/flux`,
      component: () => {
        return <FluxVersionsTable versions={Object.values(fluxVersions)} />;
      },
      visible: true,
    });
  }

If it's a temp change which will be fixed later, like commenting out code for the time being, could you please add a comment explaining if it's temporary or in which cases a developer might want to set this value to false?

[enekofb]

good shout, I wanted to remove it all together under the assumption of having a consistent experience between OSS and EE (using the multi-cluster scenario as baseline) ...

how do you feel and @jpellizzari about it?

[opudrovs]

Fine with me!

[opudrovs]

❔ When will this FIXME be addressed? Is there a follow-up issue for this or is it just "nice to do one day"?

[enekofb]

removed cause was original design to have a single component ... the design i am now behind is to just replace fluxruntime

[opudrovs]

💡 Is it possible to replace one label in labelled with Label with another word (for readability)? Maybe just labeled with or, if the word Label is required, have Label/contain Label, whatever.

[enekofb]

yup, reads better now

[enekofb]

❔ Will any of new components/hooks/functions be used in enterprise directly? I mean, should anything be added to exports in ui/index.ts?

Interesting question that clashes with my existing FE knowledge: yes, we would like to use the feature as is in EE. it is the second story on this epic weaveworks/weave-gitops-enterprise#3725

What changes needs to do in this PR to just use it in EE?

[enekofb]

❔ It's not an issue, just a minor question, related to usability (and I am not sure who will use this new UI, so don't know if it's of any importance). Since the routes of both views are slightly different:

/flux_runtime/flux
/runtime/flux

if the user toggles the feature flag and just refreshes the page (while browser stays at the same route), there will be a page not found error displayed:

Is it possible that someone will share a link to a runtime page (in a documentation, for example) and then the flag will be toggled and the users will bump into page not working? Would a redirect or keeping both routes functional help? If it does not matter (or like in "if anyone starts screaming about it, we'll figure it out then" 😅 ), never mind.

Yes, it is a bit annoying but think of a limited impact ( i think only once) as you face that situation in a scenario where there an existing WEGO installed. Given that its a configuration issue, it will be noticed by the platfrom eng or the dev doing the upgrade or configuration. The rest of the wego users, given there is a new release of the app would need to relogin and use whatever route is there via navigation so hopefully not that impacted.

Having both routes /flux_runtime and /runtime feels semantically incorrect in terms of exectations for users going to /flux_runtime and potentially seeing other non-flux controllers

My 2cts is that we could deliver as is and as proven value, we deprecate flux_runtime as a sub-set of runtime ... if not value for runtime ... then we just dont ship it for OSS (only for EE )

How do you feel about that @opudrovs ?

[enekofb]

@opudrovs re-requested review after addressing comments. PTAL 🙏

[opudrovs]

Having both routes /flux_runtime and /runtime feels semantically incorrect in terms of exectations for users going to /flux_runtime and potentially seeing other non-flux controllers

My 2cts is that we could deliver as is and as proven value, we deprecate flux_runtime as a sub-set of runtime ... if not value for runtime ... then we just dont ship it for OSS (only for EE )

@enekofb I see, thanks, fine with me.

[opudrovs]

@enekofb

Interesting question that clashes with my existing FE knowledge: yes, we would like to use the feature as is in EE. it is the second story on this epic weaveworks/weave-gitops-enterprise#3725

What changes needs to do in this PR to just use it in EE?

It's more related to our specific product setup and to how we build the OSS package (@weaveworks/weave-gitops) which is imported to enterprise not frontend in general, you frontend knowledge is fine. 😸

In order to be able to import an identifier in enterprise from OSS (I mean directly, if it's not a part of an already imported component), you need to export it from OSS in ui/index.ts. Just import them to index.ts and add them to the long list of exports:

weave-gitops/ui/index.ts

Line 143 in 78d5f6d

export {

I am not sure what should be added to the exports, it depends on what will be imported when the new components/function are used in enterprise. So, feel free to add the exports now or in the next issue.

[opudrovs]

Re-tested it with the flag on and off, LGTM!

[enekofb]

@enekofb

Interesting question that clashes with my existing FE knowledge: yes, we would like to use the feature as is in EE. it is the second story on this epic weaveworks/weave-gitops-enterprise#3725

What changes needs to do in this PR to just use it in EE?

It's more related to our specific product setup and to how we build the OSS package (@weaveworks/weave-gitops) which is imported to enterprise not frontend in general, you frontend knowledge is fine. 😸

In order to be able to import an identifier in enterprise from OSS (I mean directly, if it's not a part of an already imported component), you need to export it from OSS in ui/index.ts. Just import them to index.ts and add them to the long list of exports:

weave-gitops/ui/index.ts

Line 143 in 78d5f6d

export {

I am not sure what should be added to the exports, it depends on what will be imported when the new components/function are used in enterprise. So, feel free to add the exports now or in the next issue.

done!
93d08fe