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

Replace slash and period with underscore for label names #237

Merged
merged 1 commit into from
Aug 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 30 additions & 1 deletion pkg/logging/labels.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,44 @@
package logging

import (
"fmt"
"strings"

"github.com/go-logr/logr"
)

// WithLabels decorates a logr.Logger so that any log entries contain all
// labels which keys are prefix by labelKeyPrefix
func WithLabels(logger logr.Logger, labels map[string]string, labelKeyPrefix string) logr.Logger {
for key, value := range labels {
logger = logger.WithValues(labelKeyPrefix+key, value)
logger = logger.WithValues(
fmt.Sprintf(
"%s%s",
labelKeyPrefix,
linearLabelKey(key),
),
value,
)
}

return logger
}

// linearLabelKey reduces a given label key to a linear form such that
// an incoming label key will have underscores in place of periods and
// forward slashes. This does have the potential to have multiple keys
// map down onto a single key. This is not likely to occur, and is
// likely avoidable for most installations.
//
// This behaviour matches the behaviour seen in some other systems
// such as Prometheus where labels are reduced to a alphanumeric
// string with underscore spacers.
//
// e.g. app.kubernetes.io/instance would become
// app_kubernetes_io_instance
func linearLabelKey(labelKey string) string {
labelKey = strings.ReplaceAll(labelKey, "/", "_")
labelKey = strings.ReplaceAll(labelKey, ".", "_")
Copy link
Contributor

Choose a reason for hiding this comment

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

I wondered if we should be doing the same for dashes

https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set

But actually it seems like ES would be fine with these, so no issue there


return labelKey
}
10 changes: 7 additions & 3 deletions pkg/workloads/console/runner/integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ func newNamespace(name string) corev1.Namespace {
}
}

const (
shortTimeout time.Duration = 30 * time.Millisecond
)

func newConsoleTemplate(namespace, name string, labels map[string]string) workloadsv1alpha1.ConsoleTemplate {
return workloadsv1alpha1.ConsoleTemplate{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -276,7 +280,7 @@ var _ = Describe("Runner", func() {

Context("When console phase is Pending", func() {
It("Fails with a timeout waiting", func() {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond)
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
defer cancel()
_, err := consoleRunner.WaitUntilReady(ctx, console, true)

Expand Down Expand Up @@ -431,7 +435,7 @@ var _ = Describe("Runner", func() {
rb.Subjects = nil
mustCreateRoleBinding(rb)

ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond)
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
defer cancel()
_, err := consoleRunner.WaitUntilReady(ctx, csl, true)

Expand Down Expand Up @@ -482,7 +486,7 @@ var _ = Describe("Runner", func() {
},
)

ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond)
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
defer cancel()
_, err := consoleRunner.WaitUntilReady(ctx, csl, true)

Expand Down