Containers endpoints (routes/ingresses)

[l0rd]

Current devfile 1.0 container.endpoints syntax (link to the doc):

components:
  - alias: maven
    type: dockerimage
    image: eclipe/maven-jdk8:latest
    endpoints:
      - name: maven-server
        port: 3101
        attributes:
          protocol: http
          secure: 'true'
          public: 'true'
          discoverable: 'false'

Current syntax of che plugins (link to the doc):

spec:
  endpoints:
   -  name: "theia"
      public: true
      targetPort: 3100
      attributes:
        protocol: http
        type: ide
        secure: true
        cookiesAuthEnabled: true
        discoverable: false
  containers:
   - name: theia-ide
     image: "quay.io/eclipse/che-theia:7.10.0"
     ports:
         - exposedPort: 3100

[l0rd]

cc @davidfestal @kadel

[davidfestal]

In the existing initial proposal of devfile 2.0, I already tried to harmonize these differences, based on the definitions in the Che plugins meta.yaml schema, which seems to be more precise and complete, especially providing real boolean fields, instead of non-validated strings for everything.

I had also added a slight change (mentioned below), to distinguish URL scheme protocol (http, https, ws...), which I renamed scheme and the transport protocol (tcp, udp, ...) which I named protocol.

In K8S, container port protocol is the fact the transport protocol.

However this is subject to approval / alternate proposals. For example to maintain compatibility with the 1.0 definition, maybe we could also have protocol and transportProtocol ?

          endpoints:
            - name: term-websockets
              targetPort: 4000
              configuration:    <-- change from the devfile 1.0, to separate the schema-validated configuration from additional free-form *attributes*
                cookiesAuthEnabled: false
                protocol: tcp     <--- change from the dev 1.0, to distinguish between URI protocol and transport protocol 
                scheme: ws     <--- change from the dev 1.0, to distinguish between URI protocol and transport protocol 
                type: terminal
                discoverable: false
                path: "/apath"
                public: true <--- If `public` is the default (which is the case afaik), maybe the field should be `private`, and thus naturally `false` by default 
                secure: true
              attributes:
                afreeFormAttribute: "for-a-given-implementation"

[sleshchenko]

public: true <--- If public is the default (which is the case afaik), maybe the field should be private, and thus naturally false by default

We need to clarify different levels of availability we need/are able to provide:

1. Route/Ingress - public, available outside of the K8s/OS cluster;
2. Service - internal, available within namespace/cluster depending on NetworkPolicies;
3. Localhost - maybe private - we propagate like endpoint metainfo http://localhost:4323/api/petclinic that is can be consumed by theia for example, and since webview proxies request, it can be opened there(not sure about that assumption), but it's not available outside of pod and a separate browser tab.

cookiesAuthEnabled: false

Just to share that it's not something provided by default by authentication proxies, but it's what we implemented in che-jwtproxy to make sure user is aware of CSRF security vulnerability. false by default - client by itself must send token in header, if true - proxy should set cookies and no need to set token from client side, but user must make sure that there is no modifying REST API exposed. Like with theia, only webresources and WebSocket API is available.

[l0rd]

cookiesAuthEnabled: false
type: terminal

@davidfestal @sleshchenko I would move it to attributes

3. Localhost - maybe private - we propagate like endpoint metainfo

@sleshchenko this scenario should be supported with the combination of public: false (no route/ingress) and discoverable: false (no service).

configuration: <-- change from the devfile 1.0, to separate the schema-validated configuration from additional free-form attributes

It makes sense. Originally we wanted to have endpoints fields to be consistent with k8s services and have a subsection to be consistent with ingress fields. I think we should review this. For instance the protocol field confusion is related to this and we should make it clearer.

@davidfestal something that I would prefer is to have endpoints under containers/kubernetes/ and not at the same level as it is devfile 1.0.0. We should discuss that as well.

[kadel]

How is public and discoverable different?

[sparkoo]

@kadel public is accessible from outside with route/ingress. discoverable is service with defined name, so it is accessible inside cluster with predictable dns name.

[amisevsk]

There is a bit of inconsistency between discoverable and public here -- discoverable isn't just "create a service", it also constrains how that service is named. I think that the exposure level for an endpoint needs to be refined from devfile 1.0, as we currently have three properties that mostly encompass one another, but do not do so completely:

• public: A service and a route/ingress is created for this endpoint. Names are generated
• discoverable: A service is created, with name matching endpoint name.
• both false: No service, no route/ingress.

Since discoverable is almost a subproperty of public, what about merging the three properties into one:

endpoints:
  - term-websockets
    targetPort: 4000
    exposure: [public|internal|private]

where public = create route+service, internal = create service only, private = meant for localhost.

The current implementation of discoverable is a little problematic, as it can cause name collisions. I would prefer the "defined dns name" functionality to be made explicit with something like generateName: false.

[kadel]

@kadel public is accessible from outside with route/ingress. discoverable is service with defined name, so it is accessible inside cluster with predictable dns name.

So if I translate it to the Kubernetes resources:

• public: true = create Route/Ingress
• discoverable: true = create Service

So that will also mean that public: true applies discoverable: true.
public: true, discoverable: false would not work and it should be invalid.

[kadel]

Since discoverable is almost a subproperty of public, what about merging the three properties into one:

endpoints:
  - term-websockets
    targetPort: 4000
    exposure: [public|internal|private]

where

I like this. To me, it makes more sense and it avoids the possibility of invalid combinations of
public and discoverable

[l0rd]

Discussed with @kadel @elsony @davidfestal and we agreed that @amisevsk proposal makes all happy:

endpoints:
  - term-websockets
    targetPort: 4000
    exposure: [public|internal|none]

Note that I have changed private with none here because the difference between internal and private is not straightforward.

We discussed about the other property of a discoverable endpoint: the service name matches the endpoint name. That's something that we assumed we could ignore in the new spec.

[sparkoo]

So that will also mean that public: true applies discoverable: true.
public: true, discoverable: false would not work and it should be invalid.

@kadel public: true and discoverable: false will work. The service will be created, only it's name will be generated, so you can't reach the service with endpoint name.

[sparkoo]

We discussed about the other property of a discoverable endpoint: the service name matches the endpoint name. That's something that we assumed we could ignore in the new spec.

@l0rd You mean to have all service names generated? Or have all services named by endpoint names?

[l0rd]

@sparkoo all generated but predictable like <workspace-id>-<endpoint-name>. What do you think about it?

[metlos]

I am not sure about the none exposure:

endpoints:
  - term-websockets
    targetPort: 4000
    exposure: none

What is this trying to tell me? Why am I bothering declaring an endpoint that is not exposed?

[metlos]

@sparkoo all generated but predictable like -. What do you think about it?

If there are multiple workspaces in a single namespace we can run into name clashes or potentially exceed the name length limit (for example if we need to include workspace id in the name for disambiguation purposes).

[sparkoo]

@sparkoo all generated but predictable like -. What do you think about it?

@l0rd what does it mean "predictable generated"? I guess main usecase for discoverable endpoint is server<>database communication, so I can have single configuration of the server with database url. Having database's endpoint generated and have to update the app configuration at every workspace startup is pain for the user. On the other hand, like @metlos mentioned, with static names there is high chance of name clashes when there are multiple workspaces in single namespace.

I wonder if there is any compromise solution, like having the service name in env variable or generate the service only once so it will be same on the next start...

[amisevsk]

I guess main usecase for discoverable endpoint is server<>database communication, so I can have single configuration of the server with database url.

Given that current workspaces use a single deployment, why can't we just use localhost for this case? The drawbacks to statically named services seem quite large to me.

[amisevsk]

Tested an updated java-mysql devfile (since that uses a discoverable db service), and it seems to work without issue.

The diff is:

• Remove discoverable endpoint
• Update command that depends on endpoint to use localhost:3306

devfile.yaml

metadata:
  name: database-localhost-test
projects:
  - name: web-java-spring-petclinic
    source:
      location: 'https://github.com/spring-projects/spring-petclinic.git'
      type: git
      branch: master
attributes:
  persistVolumes: 'false'
components:
  - id: redhat/java/latest
    memoryLimit: 1280Mi
    type: chePlugin
  - mountSources: true
    endpoints:
      - name: 8080/tcp
        port: 8080
    memoryLimit: 1000Mi
    type: dockerimage
    volumes:
      - name: m2
        containerPath: /home/user/.m2
    image: 'quay.io/eclipse/che-java8-maven:nightly'
    alias: tools
    env:
      - value: ''
        name: MAVEN_CONFIG
      - value: >-
          -XX:MaxRAMPercentage=50.0 -XX:+UseParallelGC -XX:MinHeapFreeRatio=10
          -XX:MaxHeapFreeRatio=20 -XX:GCTimeRatio=4
          -XX:AdaptiveSizePolicyWeight=90 -Dsun.zip.disableMemoryMapping=true
          -Xms20m -Djava.security.egd=file:/dev/./urandom -Duser.home=/home/user
        name: JAVA_OPTS
      - value: $(JAVA_OPTS)
        name: MAVEN_OPTS
  - mountSources: true
    memoryLimit: 300Mi
    type: dockerimage
    image: docker.io/centos/mysql-57-centos7
    alias: mysql
    env:
      - value: petclinic
        name: MYSQL_USER
      - value: petclinic
        name: MYSQL_PASSWORD
      - value: petclinic
        name: MYSQL_DATABASE
      - value: '$(echo ${0})\\$'
        name: PS1
apiVersion: 1.0.0
commands:
  - name: maven build
    actions:
      - workdir: '${CHE_PROJECTS_ROOT}/web-java-spring-petclinic'
        type: exec
        command: mvn clean install
        component: tools
  - name: run webapp
    actions:
      - workdir: '${CHE_PROJECTS_ROOT}/web-java-spring-petclinic'
        type: exec
        command: >
          SPRING_DATASOURCE_URL=jdbc:mysql://localhost:3306/petclinic \

          SPRING_DATASOURCE_USERNAME=petclinic \

          SPRING_DATASOURCE_PASSWORD=petclinic \

          java -jar -Dspring.profiles.active=mysql \

          -Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=n,address=5005
          \

          target/*.jar
        component: tools
  - name: prepare database
    actions:
      - type: exec
        command: >
          /opt/rh/rh-mysql57/root/usr/bin/mysql -u root <
          ${CHE_PROJECTS_ROOT}/web-java-spring-petclinic/src/main/resources/db/mysql/user.sql
          &&

          /opt/rh/rh-mysql57/root/usr/bin/mysql -u root petclinic <
          ${CHE_PROJECTS_ROOT}/web-java-spring-petclinic/src/main/resources/db/mysql/schema.sql
          &&

          echo -e "\e[32mDone.\e[0m Database petclinic was configured!"
        component: mysql
  - name: Debug remote java application
    actions:
      - referenceContent: |
          {
          "version": "0.2.0",
          "configurations": [
            {
              "type": "java",
              "name": "Debug (Attach) - Remote",
              "request": "attach",
              "hostName": "localhost",
              "port": 5005
            }]
          }
        type: vscode-launch

[sparkoo]

@amisevsk that's good. Only issue I have with this is that if now we say that we run everything in single pod, which is currently only implemetantion detail imho, it will become our API and we're stuck on this forever. Maybe it's ok.

[davidfestal]

We could do exactly the same as what @amisevsk did but with a predictable service name. Even environment variables that contain the service name for each endpoint (that has a service created) could be made available to the devfile.

[kadel]

What if we keep endpoints as

endpoints:
  - name: term-websockets
    targetPort: 4000
    exposure: [public|internal|none]
    protocol: tcp    <--- OPTIONAL change from the dev 1.0, to distinguish between URI protocol and transport protocol 
    path: "/apath"
    secure: true
    annotations:    <-- change from the devfile 1.0, to separate the schema-validated configuration from additional free-form *attributes*
        cookiesAuthEnabled: false
        scheme: ws     <--- change from the dev 1.0, to distinguish between URI protocol and transport protocol 
        type: terminal
        afreeFormAttribute: "for-a-given-implementation"

But we don't use name for naming k8s Services. It will be just an endpoint identification in devfile.
We can then autogenerate environment variables in a well-know format. For example, using pattern $DEVFILE_ENDPOINT_<containerName>_<endpointName>.
This env variable could be used in commands or even in the container env section.

Using this system we don't need predictable Service names and at the same time use the single configuration.

schemaVersion: "2.0.0"
metadata:
  name: test
components:
  - container:
      name: tools
      image: image
      env:
        - name: DB_HOST
          value: ${DEVFILE_ENDPOINT_MYSQL_HTTP}
  - container:
      image: docker.io/centos/mysql-57-centos7
      name: mysql
      endpoints:
        - name: http
          targetPort: 4000
commands:
  - exec:
      id: run app
      commandLine: > 
         SPRING_DATASOURCE_URL=jdbc:mysql://${DEVFILE_ENDPOINT_MYSQL_HTTP}:3306/petclinic \
         java -jar target/*.jar
     

[amisevsk]

@kadel The original use-case that 'discoverable' endpoints were designed to address is perhaps more of a "ease of use"/"ease of transition" to workspaces function.

Without discoverable endpoints, your application has to be configured to compute the URI for a service it depends on: as an example, you could have a backend application that talks to a database -- without predictable service names, the bootstrap process has to figure out where the database is. If I imagine my "prod" app running in namespace A, where the DB is on a known service, it's potentially much easier to duplicate that setup in namespace B with discoverable services. I don't think many workspace are actually impacted by this though.

It's easily worked around in the Eclipse Che implementation, since all containers run in one pod and thus share a network, but a workspace where the database and development component live in separate pods makes this more complicated.

I agree that setting predictable environment variables is probably the way to go (IIRC we do this already in Che), but I wanted to share some historical context.