Simplify Eclipse Che configuration format (CheCluster API v2)

[tolusha]

Is your task related to a problem? Please describe

After switching to DevWorkspace engine we might need to drop some fields.
Some fields are deprecated already, other are required to be renamed.

Describe the solution you'd like

Collect all possible changes in CheCluster CR

Additional context

This is how the new configuration sections looks like

> devEnvironments
    > defaultPlugins
    > defaultNamespace
    > storage
    > nodeSelector
    > tolerations
    > trustedCerts
        > gitTrustedCertsConfigMapName
> components
    > devWorkspace
    > dashboard
    > cheServer
        > clusterRoles
        > deployment
        > loglevel
        > debug
        > proxy
        > extraProperties
    > database
    > pluginRegistry
    > devfileRegistry
    > imagePuller
    > metrics
> networking
    > labels
    > annotations
    > domain
    > hostname
    > tlsSecretName
    > auth
> containerRegistry

Release Notes Text

CheCluster custom resource definition has been updated to reflect that changes introduced with the switch to the DevWorkspaces. The result is a shorter and simpler configuration file to maintain for Eclipse Che administrators.

[tolusha]

I. Delete
Make no sense in DevWorkspace world:

• status.KeycloakProvisoned
• status.KeycloakURL
• status.OpenShiftOAuthUserCredentialsSecret
• auth.identityProviderAdminUserName
• auth.identityProviderPassword
• auth.identityProviderSecret
• auth.identityProviderPostgresPassword
• auth.identityProviderPostgresSecret
• auth.InitialOpenShiftOAuthUser
• auth.ExternalIdentityProvider
• auth.IdentityProviderRealm
• auth.IdentityProviderClientId
• auth.OpenShiftoAuth (it will be always true)
• auth.NativeUserMode (it will be always true)
• auth.UpdateAdminPassword
• auth.IdentityProviderImage
• auth.IdentityProviderImagePullPolicy
• auth.IdentityProviderContainerResources
• auth.IdentityProviderRoute
• auth.IdentityProviderIngress
• auth.debug

Deprecated:

• server.allowUserDefinedWorkspaceNamespaces
• server.selfSignedCert
• server.cheFlavor
• server.tlsSupport
• server.useInternalClusterSVCNames
• server.disableInternalClusterSVCNames
• server.devfileRegistryUrl
• auth.gatewayHeaderRewriteSidecarImage
• k8s.ingressStrategy
• k8s.singleHostExposureType

Single-host strategy will be the only one supported. We can remove fields related to multi-host strategy.

• server.ServerExposureStrategy
• server.DashboardRoute
• server.PluginRegistryRoute
• server.DevfileRegistryRoute
• server.DashboardIngress
• server.PluginRegistryIngress
• server.DevfileRegistreIngress

Duplication:

• server.proxyUser, server.proxyPassword, in favor of server.proxySecret
• database.chePostgresPassword, database.chePostgresUser in favor of databse.chePostgresSecret

Legacy:

• server.ServerTrustStoreConfigMapName
• server.CheImageTag in favor of server.CheImage

II. Move/Rename/Refactor

• remove prefix CheServer from server.CheServer* fields
• move server.DevfileRegistry* -> devfileregistry.*
• move server.PluginRegistry* -> pluginregistry.*
• move server.Dashboard* -> dashboard.*
• makeserver.CheHost as a part of server.CheServerRoute and server.CheServerIngress
• make server.CheHostTLSSecret as part of server.CheServerRoute
• make k8s.TlsSecretName as part of server.CheServerIngress
• make k8s.IngressDomain as part of server.CheServerIngress

III. Registries

• delete pluginRegistryUrl (was used to configure external plugin registry url)
• delete devfileRegistryUrl (was used to configure external devfile registry url)
• deleteserver.ExternalDevfileRegistry
• delete server.ExternalPluginRegistry
• add devfileregistry.enable to deploy dedicated devfile registry
• add pluginregistry.enable to deploy dedicated plugin registry
• add pluginregistry.ExternalPluginRegistry to configure external plugin registries regardless dedicated one
• add devfileregistry.ExternalDevfileRegistry to configure external plugin registries regardless dedicated one

[mmorhun]

@tolusha great work!

move server.CheHostTLSSecret into server.CheServerRoute

I'd move into CheDashboardEndpointTLSSecret as Route is Openshift specific.

move k8s.TlsSecretName into server.CheServerIngress

The name is not clear. From a first shight it might be percepted as ingress name to use for Che server. What about server.EndpointTLSSecret? (than we might have it for both Kubernetes and Openshift)

move k8s.IngressDomain into server.CheServerIngress

Not clear. I'd like to keep domain word, and even old name, but moved to server section maybe.

server.ExternalDevfileRegistry -> devfileregistry.enable

if I set devfileregistry.enable to false will it disable it at all 😄 ?
The same for plugin registry. I think word external is mandatory in the name.

Single-host strategy will be the only one supported. We can remove fields related to multi-host strategy.

If we delete multi host, so does it makes sense to keep both CheTLSSecret and CheHostTLSSecret?

[AndrienkoAleksandr]

auth.UpdateAdminPassword

Do we have the same feature for Dex?

[mmorhun]

Also, it's time to get rid of self-signed-certificate secret, isn't it?

[AndrienkoAleksandr]

server.ExternalDevfileRegistry -> devfileregistry.enable
server.ExternalPluginRegistry -> pluginregistry.enable

Looks not clear for me

[tolusha]

- make`server.CheHost` as a part of `server.CheServerRoute` and `server.CheServerIngress`
- make `server.CheHostTLSSecret` as part of `server.CheServerRoute`
- make `k8s.TlsSecretName` as part of `server.CheServerIngress`
- make `k8s.IngressDomain` as part of `server.CheServerIngress`

Hostname, secret and domain basically is part of of route(ingress). So, it is logically to move them into that structure.

auth.UpdateAdminPassword

Only works for Keycloak

server.ExternalDevfileRegistry -> devfileregistry.enable
server.ExternalPluginRegistry -> pluginregistry.enable

ExternalDevfileRegistry controls whether or not deploy dedicated registry despite the fact that it contains external in its name. Beside this there is ExternalDevfileRegistries fields which is responsible for configuring external devfile registries.
So, my suggestion is the following:

1. Remove server.ExternalDevfileRegistry field
2. Add devfileregistry.enable to control deploying dedicated registry.
3. Keep devfileregsitry.ExternalDevfileRegistries to configure external devfile registries regardless dedicated one.

The same true for plugin registry

[mmorhun]

I'd also improve server.gitSelfSignedCert

[mmorhun]

ExternalDevfileRegistry controls whether or not deploy dedicated registry despite the fact that it contains external in its name. Beside this there is ExternalDevfileRegistries fields which is responsible for configuring external devfile registries.
So, my suggestion is the following:

To which registries will the value of, for example, DevfileRegistryCpuLimit be applied? It is not clear.

2. Add devfileregistry.enable to control deploying dedicated registry.

Maybe name it devfileregistry.InternalRegistryEnabled ?

[AndrienkoAleksandr]

server.useInternalClusterSVCNames

I think this feature can be still usefull for plugin and devfile registry links and maybe for dashboard deployment.

[tolusha]

It is deprecated in favor of DisableInternalClusterSVCNames

[tolusha]

Related issue #20759

[tolusha]

spec:
  server:
    deployment: <deployment>
    route: <route>
    ingress: <ingress>
    logLevel: <string>
    debug: <string>
    clusterRoles: <[]string>
    workspaceServiceAccountClusterRole: <[]string>
    workspaceNamespaceName: <string>
    gitSelfSignedSecretRef: <string>
    airGapContainerRegistryHostname: <string>
    airGapContainerRegistryOrganization: <string>
    proxy: <proxy>
    customCheProperties: <map[string]string>
    workspacesDefaultPlugins: <[]workspacesDefaultPlugins>
  storage:
    pvc: <pvc>
    pvcJobsImage: <string>
    pvcStrategy: <string>
    preCreateSubPaths: <string>
  dashboard:
    deployment: <deployment>
  pluginRegistry:
    disableInternalRegistry: <bool>
    deployment: <deployment>
    externalPluginRegistries: <externalPluginRegistries>
  devfileRegistry:
    disableInternalRegistry: <bool>
    deployment: <deployment>
    externalDevfileRegistries: <externalDevfileRegistries>
  auth:
    identityProviderURL: <string>
    oAuthClientName: <string>
    oAuthSecretRef: <string>
    gateway: <gateway>
  database:
    externalDb: <bool>
    deployment: <deployment>
    pvc: <pvc>
    postgresDb: <string>
    postgresHostName: <string>
    postgresPort: <string>
    postgresCredentialsSecretRef: <string>
    postgresVersion: <string>
  metrics:
    enable: <bool>
  imagePuller:
    enable: <bool>
    spec: <...>
  devWorkspace:
    deployment: <deployment>
status:
  cheClusterRunning: <string>
  cheVersion: <string>
  cheURL: <string>
  devfileRegistryURL: <string>
  pluginRegistryURL: <string>
  message: <string>
  reason: <string>
  helpLink: <string>
  devworkspaceStatus: <..>
----------------------------------
deployment:
  container:
    image: <string>
    imagePullPolicy: <string>
    resources:
      limits:
        cpu: <string>
        memory: <string>
      requests:
        cpu: <string>
        memory: <string>
  securityContext:
    runAsUser: <string>
    fsGroup: <string>

gateway:
  image: <string>
  configSidecarImage: <string>
  authenticationSidecarImage: <string>
  authorizationSidecarImage: <string>
  configMapLabels: <map[string]string>

route:
  labels: <map[string]string>
  annotations: <map[string]string>
  domain: <string>
  host: <string>
  tlsSecretRef: <string>

ingress:
  class: <string>
  labels: <map[string]string>
  annotations: <map[string]string>
  domain: <string>
  host: <string>
  tlsSecretRef: <string>

proxy:
  url: <string>
  port: <string>
  nonProxyHosts: <map[string]string>
  credentialsSecretRef: <string>

pvc:
  claimSize: <string>
  storageClass: <string>

externalDevfileRegistries: <[]externalDevfileRegistry>

externalDevfileRegistry:
  url: string

externalPluginRegistries: <[]externalPluginRegistry>

externalPluginRegistry:
  url: string

[l0rd]

A few comments:

1. Wouldn't it better to move server.route and server.ingress to auth.gateway?

2. Shouldn't we rename server.workspaceClusterRole as server.workspaceServiceAccountClusterRole?

3. Shouldn't we rename server.workspaceNamespaceDefault as server.workspaceNamespaceName? Calling it Default means that it can be customized but that's not possible anymore.

4. Do we need enable for the registries? If I specify:

spec:
  pluginRegistry:
    externalPluginRegistries: <externalPluginRegistries>

I would expect to be disabled by default. I should not need to set the extra field enabled=false.

5. In general we should make sure that for booleans the default is false. If the default for enabled is true (registries I guess) we should replace it with disabled. if the default is enabled=false (metrics, imagepuller and externaldb) then we should not change it. Anyway if we remove the enable field from the registries as suggested in point 4. we should be fine.

6. Do we support multiple external plugin registries?

[tolusha]

1. I don't have a good answer. I would move it rather to dashboard.
2. ok
3. ok
4. Yes. We need this at least for devfile registry. We can configure external registries urls a log with bundled one.
5. I make sense to rename enable -> disabled in some cases
6. Not yet. But I would like to have similar functionality with devfile registry.

[l0rd]

1. I don't have a good answer. I would move it rather to dashboard.

Is the route/ingress used by the dashboard only or also used by other components (workspaces etc...)? cc @sparkoo

[l0rd]

@tolusha I have moved this issue to 7.48

[tolusha]

Sounds good. I have to write an email about incoming changes.

[l0rd]

@tolusha can we close this issue now?

[tolusha]

The PR [1] is not merged yet. Here is a new CheCluster API v2 [2]
If you don't have any concerns, then pls approve. I will merge once all checks are green.
[1] eclipse-che/che-operator#1324
[2] https://github.com/eclipse-che/che-operator/blob/20758/api/v2/checluster_types.go

[nickboldt]

postponed to 7.50; plan is to merge this as soon as 7.49.x branches are created

[nickboldt]

Recent commits to main (7.50) have broken downstream devspaces 3.1 operator bundle builds.

See https://issues.redhat.com/browse/CRW-3064

[devstudio-release]

sync'd to Red Hat JIRA https://issues.redhat.com/browse/CRW-3090

[Divine1]

Recent commits to main (7.50) have broken downstream devspaces 3.1 operator bundle builds.

See https://issues.redhat.com/browse/CRW-3064

@nickboldt

currently i'm using below chectl version. if i upgrade to chectl 7.5.0, will it cause break in my eclipse-che setup?

[l0rd]

@Divine1 you should be able to upgrade. The comment above is about our CI that builds OpenShift Dev Spaces (a Red Hat product) from Eclipse Che sources.

[Divine1]

@Divine1 you should be able to upgrade. The comment above is about our CI that builds OpenShift Dev Spaces (a Red Hat product) from Eclipse Che sources.

Thank you @l0rd