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

Enable securing query component by a sidecar #1105

Closed
wants to merge 8 commits into from
Closed

Enable securing query component by a sidecar #1105

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f9ca42a
Enable securing query component by a sidecar
basch255 Jun 26, 2020
039826d
securing ui, adapting oauth-proxy
basch255 Jul 3, 2020
2d2211e
securing ui, adapting oauth-proxy
Jul 3, 2020
57e42c3
securing ui, adapting oauth-proxy
basch255 Jul 3, 2020
f5fddec
Securing UI, adapting oauth-proxy
nilsboeh Jul 8, 2020
c83de8c
Securing UI, adapting oauth-proxy
nilsboeh Jul 9, 2020
bf2cb92
Oauth-proxy expanded and bugs fixed
basch255 Jul 17, 2020
bb398d7
Merge branch 'master' into securing-query-component-with-a-sidecar
basch255 Jul 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions deploy/examples/secure-query-with-keycloak-sidecar-values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
apiVersion: v1
kind: Jaeger
metadata:
name: jaeger
namespace: default
spec:
ingress:
enabled: true
security: "oauth-proxy"
query:
oauthProxy:
options:
# Client ID for OpenID server
client-id: jaeger-client
# Client secret for OpenID server.
# Confidential access type is for server-side clients that need to perform a browser login and require a client
# secret when they turn an access code into an access token, (see Access Token Request in the OAuth 2.0 spec for
# more details). This type should be used for server-side applications.
# see https://www.keycloak.org/docs/6.0/server_admin/
client-secret: 12345678-9abc-def0-1234-56789abcdef0
# URL for OpenID autoconfiguration
discovery-url: https://keycloak.example.com/auth/realms/jaeger-realm
# Enables a default denial on all requests, you have to explicitly say what is permitted (recommended)
enable-default-deny: true
# The interface definition you wish the proxy to listen, all interfaces is specified as ':<port>', unix sockets as unix://<REL_PATH>|</ABS PATH>
listen: ":9091"
# Preserve the host header of the proxied request in the upstream request
preserve-host: true
# The redirection url, essentially the site url, note: /oauth/callback is added at the end
redirection-url: "https://jaeger-gatekeeper.example.com"
# Role restrictions for URL. User must have specified role in order to access.
resources: "uri:/search*|white-listed=true"
# Skip the verification of any upstream TLS
skip-upstream-tls-verify: false
# Skip the verification of any TLS communication with the openid provider
skip-openid-provider-tls-verify: false
# URL of the service to proxy
upstream-url: "http://127.0.0.1:9090"
# Path to ths TLS certificate
tls-cert: "/certs/tls.pem"
# Path to the private key for TLS
tls-private-key: "/certs/tls-key.pem"
# Create self signed certificates for the proxy
enable-self-signed-tls: true
self-signed-tls-expiration: "87600h0m0s"
self-signed-tls-hostnames: "localhost"
resources: {}
13 changes: 13 additions & 0 deletions pkg/apis/jaegertracing/v1/jaeger_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,19 @@ type JaegerQuerySpec struct {

// +optional
JaegerCommonSpec `json:",inline,omitempty"`

// +optional
OauthProxy JaegerQueryOauthProxySpec `json:"oauthProxy,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be a pointer, so that an empty object isn't persisted in case the user does not specify it.

}

// JaegerQueryOauthProxySpec define the options to be used when deploying a sidecar to the query
// +k8s:openapi-gen=true
type JaegerQueryOauthProxySpec struct {
// +optional
Image string `json:"image,omitempty"`

// +optional
Options Options `json:"options,omitempty"`
}

// JaegerUISpec defines the options to be used to configure the UI
Expand Down
18 changes: 18 additions & 0 deletions pkg/apis/jaegertracing/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 39 additions & 1 deletion pkg/apis/jaegertracing/v1/zz_generated.openapi.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions pkg/cmd/start/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ func AddFlags(cmd *cobra.Command) {
cmd.Flags().String("jaeger-es-rollover-image", "jaegertracing/jaeger-es-rollover", "The Docker image for the Jaeger Elasticsearch Rollover")
viper.BindPFlag("jaeger-es-rollover-image", cmd.Flags().Lookup("jaeger-es-rollover-image"))

cmd.Flags().String("oauth-proxy-image", "quay.io/keycloak/keycloak-gatekeeper:10.0.0", "The Docker image location definition for the Default OAuth Proxy")
viper.BindPFlag("oauth-proxy-image", cmd.Flags().Lookup("oauth-proxy-image"))

cmd.Flags().String("openshift-oauth-proxy-image", "openshift/oauth-proxy:latest", "The Docker image location definition for the OpenShift OAuth Proxy")
viper.BindPFlag("openshift-oauth-proxy-image", cmd.Flags().Lookup("openshift-oauth-proxy-image"))

Expand Down
51 changes: 37 additions & 14 deletions pkg/inject/oauth_proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,46 @@ const defaultProxySecret = "ncNDoqLGrayxXzxTn5ANbOXZp3qXd0LA"

// OAuthProxy injects an appropriate proxy into the given deployment
func OAuthProxy(jaeger *v1.Jaeger, dep *appsv1.Deployment) *appsv1.Deployment {
if jaeger.Spec.Ingress.Security != v1.IngressSecurityOAuthProxy {
return dep
if jaeger.Spec.Ingress.Security == v1.IngressSecurityOAuthProxy {
Copy link
Contributor

Choose a reason for hiding this comment

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

The old code was doing a short-circuit here: if the security isn't OAuth Proxy, then just return the dep. Everything else would be handled outside of an if statement. I find that approach more readable, but you can keep it this way if you prefer it.

dep.Spec.Template.Spec.ServiceAccountName = account.OAuthProxyAccountNameFor(jaeger)

if viper.GetString("platform") == v1.FlagPlatformOpenShift {
dep.Spec.Template.Spec.Containers = append(dep.Spec.Template.Spec.Containers, getOpenShiftOAuthProxyContainer(jaeger))
dep.Spec.Template.Spec.Volumes = append(dep.Spec.Template.Spec.Volumes, corev1.Volume{
Name: service.GetTLSSecretNameForQueryService(jaeger),
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: service.GetTLSSecretNameForQueryService(jaeger),
},
},
})
} else {
dep.Spec.Template.Spec.Containers = append(dep.Spec.Template.Spec.Containers, getOAuthProxyContainer(jaeger))
}
}

dep.Spec.Template.Spec.ServiceAccountName = account.OAuthProxyAccountNameFor(jaeger)
dep.Spec.Template.Spec.Containers = append(dep.Spec.Template.Spec.Containers, getOAuthProxyContainer(jaeger))
dep.Spec.Template.Spec.Volumes = append(dep.Spec.Template.Spec.Volumes, corev1.Volume{
Name: service.GetTLSSecretNameForQueryService(jaeger),
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: service.GetTLSSecretNameForQueryService(jaeger),
},
},
})
return dep
}

func getOAuthProxyContainer(jaeger *v1.Jaeger) corev1.Container {
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing I just realized while reviewing this PR: we want the generic OAuth feature to allow users to specify their own OAuth Proxies, so that we are not tied to Keycloak specifically.

With that in mind, the first consumer that overrides the default OAuth Proxy could be the operator itself, when running on OpenShift. Meaning: the code path for the generic OAuth proxy could be used for both OpenShift and non-OpenShift, with the OpenShift path only customizing the OAuth parts (image/args).

I know it might sound a bit frustrating to hear this at this point in the PR, but it just clicked with me. For some reason, I believe you'll also realize that :)

commonSpec := util.Merge([]v1.JaegerCommonSpec{jaeger.Spec.Ingress.JaegerCommonSpec, jaeger.Spec.JaegerCommonSpec})

args := jaeger.Spec.Query.OauthProxy.Options.ToArgs()
sort.Strings(args)

return corev1.Container{
Image: util.ImageName(jaeger.Spec.Query.OauthProxy.Image, "oauth-proxy-image"),
Name: "oauth-proxy",
Args: args,
Ports: []corev1.ContainerPort{
{
ContainerPort: 9091,
Name: "public",
},
},
Resources: commonSpec.Resources,
}
}

func proxyInitArguments(jaeger *v1.Jaeger) []string {
secret, err := util.GenerateProxySecret()
if err != nil {
Expand All @@ -55,7 +78,7 @@ func proxyInitArguments(jaeger *v1.Jaeger) []string {
return args
}

func getOAuthProxyContainer(jaeger *v1.Jaeger) corev1.Container {
func getOpenShiftOAuthProxyContainer(jaeger *v1.Jaeger) corev1.Container {
commonSpec := util.Merge([]v1.JaegerCommonSpec{jaeger.Spec.Ingress.JaegerCommonSpec, jaeger.Spec.JaegerCommonSpec})
ca.Update(jaeger, commonSpec)

Expand Down
Loading