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.

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

Secure Jaeger UI #681

Merged
4 commits merged into from Sep 24, 2018
Merged

Secure Jaeger UI #681

4 commits merged into from Sep 24, 2018

Conversation

ghost
Copy link

@ghost ghost commented Sep 13, 2018

Description

Changes proposed in this pull request:

  • Add a "keycloack proxy" as a sidecar to Jaeger
  • Expose Jaeger UI only through its "keycloack" sidecar
  • Extend "dex" to support a new static client for Jaeger UI integration

Related issue(s)

Implements #356

@ghost ghost changed the title Secure Jaeger UI (#356) Secure Jaeger UI Sep 13, 2018
@@ -8,14 +7,26 @@ spec:
hosts:
- jaeger.{{ .Values.global.domainName }}
gateways:
- {{ .Values.global.istio.gateway.name }}
- core-{{ .Chart.Name }}-gateway
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

We have defined a new “security context” only for Jaeger UI which is independent of the others already existing. Therefore, we have a new “static client” for dex, see the related PR.

Jaeger UI will be used by developers in addition to admins or support. Compromising it should not imply that the others are compromised too.

resources/core/charts/jaeger/templates/virtualservice.yaml Outdated Show resolved Hide resolved
@CLAassistant
Copy link

CLAassistant commented Sep 19, 2018

CLA assistant check
All committers have signed the CLA.

@ghost ghost added area/security Issues or PRs related to security area/tracing Issues or PRs related to the tracing module (deprecated) kind/feature Categorizes issue or PR as related to a new feature. and removed area/security Issues or PRs related to security labels Sep 19, 2018
Copy link
Contributor

@montaro montaro left a comment

Choose a reason for hiding this comment

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

Please check my latest commit

{{- if .Values.jaeger.memory }}
args: ["--memory.max-traces={{ .Values.jaeger.memory.maxTraces }}"]
{{- end }}
resources:
{{ toYaml .Values.resources | indent 12 }}
- image: quay.io/gambol99/keycloak-proxy:v2.2.1
Copy link
Contributor

Choose a reason for hiding this comment

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

I will update keycloak to the latest version 2.3.0

Copy link
Contributor

@devguyio devguyio left a comment

Choose a reason for hiding this comment

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

LGTM

@ghost ghost merged commit 1efc7b3 into kyma-project:master Sep 24, 2018
@abbi-gaurav
Copy link
Member

See also #356

@ghost ghost deleted the secure_jaeger_ui branch February 15, 2019 11:57
grischperl pushed a commit to grischperl/kyma that referenced this pull request Nov 10, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tracing Issues or PRs related to the tracing module (deprecated) kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants