The Ingress Detail pane now has an extra table which shows all the rules

[marcosdiez]

I've always felt the ingress panel has less information than necessary. So I added an extra table for rules.

It can be tested using the following container: marcosdiez/dashboard:v2.1.0h

Although the PR is not ready yet to be merged, I do accept comments on my implementation.

Things I am not really sure:

• should the new template.html and components.ts be located inside src/app/frontend/resource/discovery/ingress/rulelist/ or src/app/frontend/common/components/resourcelist ? Since rules are not a resource,, but a part of ingress, I am not sure

• what exactly should I do with ingresses that don't point to services ? For starters, I've never seen it in real life, but the specs do say it's possible.

• please notice that I "flatten" the original structure so that every row has a hostname (hence my IngressRuleFlat interface)

This is how it looks like:

[codecov]

Codecov Report

Merging #5734 (ab43d15) into master (ddcdf49) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #5734   +/-   ##
=======================================
  Coverage   44.46%   44.46%           
=======================================
  Files         214      214           
  Lines        9120     9120           
  Branches      110      110           
=======================================
  Hits         4055     4055           
  Misses       4791     4791           
  Partials      274      274           

[maciaszczykm]

Thanks for the contribution. I'll take a look on it later.

[floreks]

Things I am not really sure:

• should the new template.html and components.ts be located inside src/app/frontend/resource/discovery/ingress/rulelist/ or src/app/frontend/common/components/resourcelist ? Since rules are not a resource,, but a part of ingress, I am not sure

• what exactly should I do with ingresses that don't point to services ? For starters, I've never seen it in real life, but the specs do > say it's possible.

• please notice that I "flatten" the original structure so that every row has a hostname (hence my IngressRuleFlat interface)

1. I'd say that this should be a reusable component located in common components. It can be located directly under common/components. Similar to policyrule.
2. If there is a rule with empty service I'd simply display - instead of Service Name

As for the last point. This is a nice work already, but I'd actually extend the API to offer an endpoint that would return ingress rules only and allow reusing our generic list. We do that for most of the resources on the detail views.

We are also using the old extensions/v1beta1 API and it has to be finally updated to use networking/v1 as there are differences in the API objects there that will definitely cause some issues between versions.

[marcosdiez]

Hey, @floreks , thanks for the comments.

So, about moving this to common/components, that's trivial and I can easily do.

According to the specs, rules can't have empty services, they can have a resource backend instead, which I believe my code is handling well: https://kubernetes.io/docs/concepts/services-networking/ingress/#resource-backend

About the last point, combined with the fact that you plan to change the API, I am not sure what I should do.
a) nothing
b) implement an endpoint just for the rules and then rewrite whenever we implement the networking/v1 api ?
c) implement networking/v1, then implement the dedicated endpoint.

Since the end result is the same, maybe we want to do a) now (nothing) and c) in a separate PR, for it's a bigger change and under the hood only

what do you think?

[floreks]

About the last point, combined with the fact that you plan to change the API, I am not sure what I should do.
a) nothing
b) implement an endpoint just for the rules and then rewrite whenever we implement the networking/v1 api ?
c) implement networking/v1, then implement the dedicated endpoint.

Since the end result is the same, maybe we want to do a) now (nothing) and c) in a separate PR, for it's a bigger change and under the hood only

Changing extensions to networking is rather a small change. Adding a new endpoint is a bit bigger task. If you'd like to do it in a different PR then it is also ok for me. Nevertheless, I'd wait with this PR for the API first and then update this one to do it properly at the first attempt.

The API v2 is a completely different issue not related to the current API. We'll successively migrate functionality "in the background" to match the current one. No worries there.

[marcosdiez]

Hi @floreks

As agreed, I

• moved the rulelist folder to src/app/frontend/common/components/ingressrulelist
• changed the backend to talk to the V1 Networking API ( backend: ingress will now use the networking/v1 API instead of extensions/v1beta1 (merge only in 2022?) #5741 )

It all works well, except that not things only work on k8s >= 1.19

Maybe this change was done too early ?

[marcosdiez]

Hi again, @floreks !
What else is missing for this PR to be merged ?

[floreks]

@marcosdiez one CI check fails and you also need to rebase to the latest master as angular version has been bumped and it doesn't compile anymore.

[marcosdiez]

Hi @floreks !

I did the merge and the rebase. Somehow, though, npm run fix i18n does not modify any files. please take a look of the output:

mdiez@batman:~/go/src/github.com/kubernetes/dashboard$ npm run fix:i18n

> kubernetes-dashboard@2.1.0 fix:i18n /home/mdiez/go/src/github.com/kubernetes/dashboard
> ng extract-i18n --outFile ../i18n/messages.xlf && aio/scripts/xliffmerge.sh

✔ Browser application bundle generation complete.

Initial Chunk Files | Names         |      Size
main.js             | main          |  13.70 MB
polyfills-es5.js    | polyfills-es5 | 684.99 kB
polyfills.js        | polyfills     | 266.70 kB

                    | Initial Total |  14.63 MB

Build at: 2021-01-14T12:57:20.182Z - Hash: 1feea0eb816355899af9 - Time: 82606ms

Warning: /home/mdiez/go/src/github.com/kubernetes/dashboard/src/app/frontend/environments/environment.prod.ts is                                                                                                                     part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.


Move translation file messages.de.xlf to be merged by xliffmerge.
Move translation file messages.fr.xlf to be merged by xliffmerge.
Move translation file messages.ja.xlf to be merged by xliffmerge.
Move translation file messages.ko.xlf to be merged by xliffmerge.
Move translation file messages.zh-Hans.xlf to be merged by xliffmerge.
Move translation file messages.zh-Hant.xlf to be merged by xliffmerge.
Move translation file messages.zh-Hant-HK.xlf to be merged by xliffmerge.
Move merged file i18n/messages.de.xlf to i18n/de
Move merged file i18n/messages.fr.xlf to i18n/fr
Move merged file i18n/messages.ja.xlf to i18n/ja
Move merged file i18n/messages.ko.xlf to i18n/ko
Move merged file i18n/messages.zh-Hans.xlf to i18n/zh-Hans
Move merged file i18n/messages.zh-Hant.xlf to i18n/zh-Hant
Move merged file i18n/messages.zh-Hant-HK.xlf to i18n/zh-Hant-HK
mdiez@batman:~/go/src/github.com/kubernetes/dashboard$ git status
On branch ingress_rules
nothing added to commit but untracked files present (use "git add" to track)
mdiez@batman:~/go/src/github.com/kubernetes/dashboard$

Is that expected?
Because npm run docker:build:head is not happy ?

as a workaround I can merge my code from something older than origin/master generate the files from there and "port them back here"

[floreks]

I'll try to check later and regenerate them.

[floreks]

This can be simplified.

    return [].concat(
      ...ingressSpecRules.map(rule =>
        rule.http.paths.map(
          specPath =>
            ({
              host: rule.host || '',
              path: specPath,
            } as IngressRuleFlat)
        )
      )
    );

[marcosdiez]

That's fancy! When I grow up I want to code typescript like that.

[marcosdiez]

@floreks all requested changes were made. Thanks for the code review. I learned a lot with it!

[marcosdiez]

Hey, @floreks

I was thinking over the weekend.

Since in a few of the ingress entries we have both a hostname and a path, it would be great if I could make them links so the user could click.

My biggest problem, though, is that there is no way I can obtain the protocol (HTTP or HTTPS).

In my setup for example, I have an AWS ALB doing the HTTPS part and forwarding everything to k8s. Hence, with the exception of a special custom flag in my ingress controller saying trust that all traffic is encrypted somewhere else, nowhere else my cluster is aware of that.

How to we solve that in the UI level ?

Shall I always put HTTPS in the links in every column of the ingress controller ?
Shall I use the "://" which means the browser will use whatever the k8s dashboard use ?
Shall I add two buttons, one called HTTP and one called HTTPS so the user picks which he wants to click ?
Shall I do nothing ?

Although I don't like neither option, the 3rd seems to be less worse, but your input is appreciated !

One way or another, whatever we decide here is trivial to implement and can easily come in a different PR.

Thanks !
Marcos

[floreks]

My biggest problem, though, is that there is no way I can obtain the protocol (HTTP or HTTPS).

In my setup for example, I have an AWS ALB doing the HTTPS part and forwarding everything to k8s. Hence, with the exception of a special custom flag in my ingress controller saying trust that all traffic is encrypted somewhere else, nowhere else my cluster is aware of that.

How to we solve that in the UI level ?

Shall I always put HTTPS in the links in every column of the ingress controller ?
Shall I use the "://" which means the browser will use whatever the k8s dashboard use ?
Shall I add two buttons, one called HTTP and one called HTTPS so the user picks which he wants to click ?
Shall I do nothing ?

I'd leave it for now and create an issue to improve. We could also improve the list view and try to offer working links instead of IP endpoints. If I'd have to pick, we could probably add http by default and let the browser/website redirect user to the https endpoint automatically in case http is not supported.

[floreks]

/lgtm

[floreks]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: floreks, marcosdiez

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [floreks]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: floreks, marcosdiez

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [floreks]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment