Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

hubble-ui: allow ingress from non root / urls #23631

Merged
merged 2 commits into from
Mar 25, 2023
Merged

hubble-ui: allow ingress from non root / urls #23631

merged 2 commits into from
Mar 25, 2023

Conversation

geakstr
Copy link
Contributor

@geakstr geakstr commented Feb 8, 2023

Support the case when ingress is configured to serve hubble-ui from non default / root url (ex. /service-map).

Related hubble-ui pull request:
cilium/hubble-ui#432

Signed-off-by: Dmitry Kharitonovdmitry@isovalent.com

hubble-ui: allow ingress from non root `/` urls

@geakstr geakstr added area/helm Impacts helm charts and user deployment experience needs-backport/1.10 labels Feb 8, 2023
@geakstr geakstr self-assigned this Feb 8, 2023
@geakstr geakstr requested review from a team as code owners February 8, 2023 11:22
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 8, 2023
@geakstr geakstr requested a review from sayboras February 8, 2023 11:22
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.11.14 Feb 8, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.12.7 Feb 8, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.20 Feb 8, 2023
@geakstr geakstr requested a review from kaworu February 8, 2023 11:22
@geakstr geakstr mentioned this pull request Feb 8, 2023
Merged
@kaworu kaworu added needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch release-note/misc This PR makes changes that have no direct user impact. labels Feb 8, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 8, 2023
kaworu
kaworu requested changes Feb 8, 2023
View reviewed changes
Copy link
Member

@kaworu kaworu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helm doc CI is unhappy:

Please fix the following spelling mistakes:

  • Documentation/helm-values.rst:1168: (baseUrl) hubble.ui.baseUrl
  • Documentation/helm-values.rst:1168: (baseUrl) hubble.ui.baseUrl

If the words are not misspelled, run:
Documentation/update-spelling_wordlist.sh baseUrl

@geakstr question: would this configuration knob work without a Hubble UI release containing cilium/hubble-ui#432? If not, I think the Hubble UI version bump should be part of this PR.

install/kubernetes/cilium/values.yaml Outdated Show resolved Hide resolved
install/kubernetes/cilium/templates/hubble-ui/ingress.yaml Outdated Show resolved Hide resolved
@joestringer joestringer added this to Needs backport from master in 1.10.21 Feb 13, 2023
@joestringer joestringer removed this from Needs backport from master in 1.10.20 Feb 13, 2023
@joestringer joestringer added this to Needs backport from master in 1.11.15 Feb 13, 2023
@joestringer joestringer removed this from Needs backport from master in 1.11.14 Feb 13, 2023
@joestringer joestringer added this to Needs backport from master in 1.12.8 Feb 13, 2023
@joestringer joestringer removed this from Needs backport from master in 1.12.7 Feb 13, 2023
sayboras
sayboras approved these changes Feb 16, 2023
View reviewed changes
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any other comments, lgtm 💯

nathanjsweet
nathanjsweet approved these changes Feb 16, 2023
View reviewed changes
Copy link
Member

@nathanjsweet nathanjsweet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm for k8s

@geakstr geakstr force-pushed the pr/hubble-ui/dima/fix-ingress-custom-path-prefix branch from 0324551 to 6c258d8 Compare March 7, 2023 10:03
@qmonnet
Copy link
Member

qmonnet commented Mar 24, 2023 •
edited
Loading

The failure in ConformanceGatewayAPI is not related, another instance of #24217. I re-triggered.

@qmonnet
Copy link
Member

qmonnet commented Mar 24, 2023 •
edited
Loading

In the datapath workflow, the script contrib/scripts/kind.sh fails because it doesn't get the correct number of arguments.

This is because your branch does not contain https://github.com/cilium/cilium/pull/24250/files#diff-94e101c612113648b5f525cdcb10a19f9f1b477455992d1e79fa1e6559f1fdf7R21-R26, merged in #24250 two days ago.

This failure is unrelated to the current PR, which does not touch the datapath anyway. The test passed before the latest rebase. So I don't believe it's necessary to rebase again for this test, I'm marking as ready to merge.

@qmonnet qmonnet added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 24, 2023
@youngnick youngnick merged commit e980ca0 into master Mar 25, 2023
@youngnick youngnick deleted the pr/hubble-ui/dima/fix-ingress-custom-path-prefix branch March 25, 2023 04:54
@tklauser tklauser mentioned this pull request Mar 28, 2023
Merged
5 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.11 in 1.11.16 Mar 28, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.11 in 1.11.16 Mar 28, 2023
@tklauser tklauser mentioned this pull request Mar 28, 2023
Merged
6 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.12 in 1.12.9 Mar 28, 2023
@tklauser tklauser mentioned this pull request Mar 28, 2023
Merged
9 tasks
@tklauser tklauser added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Mar 28, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.13 in 1.13.2 Mar 28, 2023
@tklauser tklauser added backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. and removed backport-pending/1.12 labels Mar 29, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.12 to Backport done to v1.12 in 1.12.9 Mar 29, 2023
@tklauser tklauser added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.11 labels Mar 29, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.11 to Backport done to v1.11 in 1.11.16 Mar 29, 2023
@tklauser tklauser added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Mar 31, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.13 to Backport done to v1.13 in 1.13.2 Mar 31, 2023
This was referenced Apr 14, 2023
Merged
Merged
@gentoo-root gentoo-root mentioned this pull request Apr 14, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaworu kaworu kaworu approved these changes

@nathanjsweet nathanjsweet nathanjsweet approved these changes

@sayboras sayboras sayboras approved these changes

@qmonnet qmonnet qmonnet approved these changes

Assignees

@geakstr geakstr

Labels
area/helm Impacts helm charts and user deployment experience backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.10.21
Needs backport from master
1.11.16
Backport done to v1.11
1.12.9
Backport done to v1.12
1.13.2
Backport done to v1.13
cilium v1.13.2
Status: Released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@geakstr @illrill @qmonnet @kaworu @nathanjsweet @sayboras @tklauser @joestringer @youngnick