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

Add option to add authentication to Jaeger and custom tags #13728

Merged
merged 1 commit into from
Oct 11, 2022
Merged

Add option to add authentication to Jaeger and custom tags #13728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 40 additions & 5 deletions install/installer/pkg/common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func DefaultEnv(cfg *config.Config) []corev1.EnvVar {
)
}

func WorkspaceTracingEnv(context *RenderContext) (res []corev1.EnvVar) {
func WorkspaceTracingEnv(context *RenderContext, component string) (res []corev1.EnvVar) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this indirection here if we're actually keeping them the same? Could we instead consolidate on func TracingEnv(...). If the need for these to actually be different arises, we can refactor.

Copy link
Member Author

@aledbf aledbf Oct 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I don't know the reason.
ping @mrsimonemms

Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing to do with me I'm afraid. This was added by @andrew-farries around 5 months ago - 1119e55

var tracing *experimental.Tracing

_ = context.WithExperimental(func(cfg *experimental.Config) error {
Expand All @@ -122,10 +122,10 @@ func WorkspaceTracingEnv(context *RenderContext) (res []corev1.EnvVar) {
return nil
})

return tracingEnv(context, tracing)
return tracingEnv(context, component, tracing)
}

func WebappTracingEnv(context *RenderContext) (res []corev1.EnvVar) {
func WebappTracingEnv(context *RenderContext, component string) (res []corev1.EnvVar) {
var tracing *experimental.Tracing

_ = context.WithExperimental(func(cfg *experimental.Config) error {
Expand All @@ -135,10 +135,10 @@ func WebappTracingEnv(context *RenderContext) (res []corev1.EnvVar) {
return nil
})

return tracingEnv(context, tracing)
return tracingEnv(context, component, tracing)
}

func tracingEnv(context *RenderContext, tracing *experimental.Tracing) (res []corev1.EnvVar) {
func tracingEnv(context *RenderContext, component string, tracing *experimental.Tracing) (res []corev1.EnvVar) {
if context.Config.Observability.Tracing == nil {
res = append(res, corev1.EnvVar{Name: "JAEGER_DISABLED", Value: "true"})
return
Expand All @@ -154,6 +154,41 @@ func tracingEnv(context *RenderContext, tracing *experimental.Tracing) (res []co
// but would make env var composition more cumbersome.
}

if context.Config.Observability.Tracing.SecretName != nil {
res = append(res, corev1.EnvVar{
Name: "JAEGER_USER",
ValueFrom: &corev1.EnvVarSource{SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: *context.Config.Observability.Tracing.SecretName},
Key: "JAEGER_USER",
}},
})

res = append(res, corev1.EnvVar{
Name: "JAEGER_PASSWORD",
ValueFrom: &corev1.EnvVarSource{SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: *context.Config.Observability.Tracing.SecretName},
Key: "JAEGER_PASSWORD",
}},
})
}
Comment on lines +157 to +173
Copy link
Member

Choose a reason for hiding this comment

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

Why do we expose these as ENV variables? In general, mounting the secret as a volume is preferred for credentials and here we could do that if the SecretName is specified.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's how the library is configured.

gitpod/components/common-go/tracing/tracing.go

Line 42 in 45b0b45

cfg, err := jaegercfg.FromEnv()

Copy link
Member

Choose a reason for hiding this comment

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

We could construct that manually, from the information we mount - https://pkg.go.dev/github.com/uber/jaeger-client-go/config#Configuration

I'll raise an issue as a follow-up (which will likely never get done) as I don't want to block your change but we do abuse env variables a fair bit.


res = append(res, corev1.EnvVar{Name: "JAEGER_SERVICE_NAME", Value: component})

jaegerTags := []string{}
if context.Config.Metadata.InstallationShortname != "" {
jaegerTags = append(jaegerTags, fmt.Sprintf("cluster=%v", context.Config.Metadata.InstallationShortname))
}

if context.Config.Metadata.Region != "" {
jaegerTags = append(jaegerTags, fmt.Sprintf("region=%v", context.Config.Metadata.Region))
}

if len(jaegerTags) > 0 {
res = append(res,
corev1.EnvVar{Name: "JAEGER_TAGS", Value: strings.Join(jaegerTags, ",")},
)
}

samplerType := experimental.TracingSampleTypeConst
samplerParam := "1"

Expand Down
2 changes: 1 addition & 1 deletion install/installer/pkg/components/agent-smith/daemonset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func daemonset(ctx *common.RenderContext) ([]runtime.Object, error) {
}},
Env: common.CustomizeEnvvar(ctx, Component, common.MergeEnv(
common.DefaultEnv(&ctx.Config),
common.WorkspaceTracingEnv(ctx),
common.WorkspaceTracingEnv(ctx, Component),
common.NodeNameEnv(ctx),
)),
SecurityContext: &corev1.SecurityContext{
Expand Down
2 changes: 1 addition & 1 deletion install/installer/pkg/components/blobserve/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) {
},
Env: common.CustomizeEnvvar(ctx, Component, common.MergeEnv(
common.DefaultEnv(&ctx.Config),
common.WorkspaceTracingEnv(ctx),
common.WorkspaceTracingEnv(ctx, Component),
)),
VolumeMounts: []corev1.VolumeMount{{
Name: "config",
Expand Down
2 changes: 1 addition & 1 deletion install/installer/pkg/components/content-service/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) {
},
Env: common.CustomizeEnvvar(ctx, Component, common.MergeEnv(
common.DefaultEnv(&ctx.Config),
common.WorkspaceTracingEnv(ctx),
common.WorkspaceTracingEnv(ctx, Component),
[]corev1.EnvVar{{
Name: "GRPC_GO_RETRY",
Value: "on",
Expand Down
2 changes: 1 addition & 1 deletion install/installer/pkg/components/image-builder-mk3/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) {
},
Env: common.CustomizeEnvvar(ctx, Component, common.MergeEnv(
common.DefaultEnv(&ctx.Config),
common.WorkspaceTracingEnv(ctx),
common.WorkspaceTracingEnv(ctx, Component),
)),
Resources: common.ResourceRequirements(ctx, Component, Component, corev1.ResourceRequirements{
Requests: corev1.ResourceList{
Expand Down
2 changes: 1 addition & 1 deletion install/installer/pkg/components/registry-facade/daemonset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ func daemonset(ctx *common.RenderContext) ([]runtime.Object, error) {
},
Env: common.CustomizeEnvvar(ctx, Component, common.MergeEnv(
common.DefaultEnv(&ctx.Config),
common.WorkspaceTracingEnv(ctx),
common.WorkspaceTracingEnv(ctx, Component),
[]corev1.EnvVar{
{
Name: "GRPC_GO_RETRY",
Expand Down
2 changes: 1 addition & 1 deletion install/installer/pkg/components/server/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) {
env := common.MergeEnv(
common.DefaultEnv(&ctx.Config),
common.DatabaseEnv(&ctx.Config),
common.WebappTracingEnv(ctx),
common.WebappTracingEnv(ctx, Component),
common.AnalyticsEnv(&ctx.Config),
common.MessageBusEnv(&ctx.Config),
common.ConfigcatEnv(ctx),
Expand Down
2 changes: 1 addition & 1 deletion install/installer/pkg/components/ws-daemon/daemonset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ fi
}},
Env: common.CustomizeEnvvar(ctx, Component, common.MergeEnv(
common.DefaultEnv(&cfg),
common.WorkspaceTracingEnv(ctx),
common.WorkspaceTracingEnv(ctx, Component),
common.NodeNameEnv(ctx),
)),
Resources: common.ResourceRequirements(ctx, Component, Component, corev1.ResourceRequirements{Requests: corev1.ResourceList{
Expand Down
2 changes: 1 addition & 1 deletion install/installer/pkg/components/ws-manager-bridge/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) {
},
Env: common.CustomizeEnvvar(ctx, Component, common.MergeEnv(
common.DefaultEnv(&ctx.Config),
common.WorkspaceTracingEnv(ctx),
common.WorkspaceTracingEnv(ctx, Component),
common.AnalyticsEnv(&ctx.Config),
common.MessageBusEnv(&ctx.Config),
common.DatabaseEnv(&ctx.Config),
Expand Down
2 changes: 1 addition & 1 deletion install/installer/pkg/components/ws-manager/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) {
},
Env: common.CustomizeEnvvar(ctx, Component, common.MergeEnv(
common.DefaultEnv(&ctx.Config),
common.WorkspaceTracingEnv(ctx),
common.WorkspaceTracingEnv(ctx, Component),
[]corev1.EnvVar{{Name: "GRPC_GO_RETRY", Value: "on"}},
)),
VolumeMounts: []corev1.VolumeMount{
Expand Down
2 changes: 1 addition & 1 deletion install/installer/pkg/components/ws-proxy/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) {
},
Env: common.CustomizeEnvvar(ctx, Component, common.MergeEnv(
common.DefaultEnv(&ctx.Config),
common.WorkspaceTracingEnv(ctx),
common.WorkspaceTracingEnv(ctx, Component),
common.AnalyticsEnv(&ctx.Config),
)),
ReadinessProbe: &corev1.Probe{
Expand Down
3 changes: 3 additions & 0 deletions install/installer/pkg/config/v1/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ type Analytics struct {
type Tracing struct {
Endpoint *string `json:"endpoint,omitempty"`
AgentHost *string `json:"agentHost,omitempty"`
// Name of the kubernetes secret to use for Jaeger authentication
// The secret should contains two definitions: JAEGER_USER and JAEGER_PASSWORD
SecretName *string `json:"secretName,omitempty"`
}

type Database struct {
Expand Down