Skip to content

Commit d57e8b7

Browse files
committed
chore(keycloak): address review + lint after rebase
- Extract labelPartOf/labelManagedBy/labelManagedByValue constants so the import-secret labels stop tripping goconst (4th occurrence of the label keys). Fixes the lint failure blocking CI. - Instrument RenderKeycloakDefaults with an OTel span and thread ctx through from InstallFoundationalServices, per the pkg/ instrumentation convention. - Render with missingkey=error so a stray template field fails loudly instead of emitting "<no value>" into the realm config; add a guard assertion. - Expand TestRenderKeycloakDefaults to cover the security-relevant scalars (sslRequired, registrationAllowed, bruteForceProtected), the realm roles, the multivalued groups-mapper flag, the argocd client flags, and webOrigins. - Drop the dead doc path from keycloak_defaults.yaml; point at #154.
1 parent 55bfe0f commit d57e8b7

4 files changed

Lines changed: 58 additions & 15 deletions

File tree

pkg/argocd/foundational.go

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,14 @@ const (
3232

3333
// NebariFoundationalPartOf is the value of the app.kubernetes.io/part-of label for foundational resources.
3434
NebariFoundationalPartOf = "nebari-foundational"
35+
36+
// labelPartOf and labelManagedBy are the standard Kubernetes recommended
37+
// label keys applied to foundational resources NIC creates.
38+
labelPartOf = "app.kubernetes.io/part-of"
39+
labelManagedBy = "app.kubernetes.io/managed-by"
40+
41+
// labelManagedByValue marks resources as managed by NIC.
42+
labelManagedByValue = "nebari-infrastructure-core"
3543
)
3644

3745
// FoundationalConfig holds configuration for foundational services
@@ -147,7 +155,7 @@ func InstallFoundationalServices(ctx context.Context, cfg *config.NebariConfig,
147155
if domain == "" {
148156
domain = "nebari.local"
149157
}
150-
importContent, err := RenderKeycloakDefaults(TemplateData{Domain: domain})
158+
importContent, err := RenderKeycloakDefaults(ctx, TemplateData{Domain: domain})
151159
if err != nil {
152160
span.RecordError(err)
153161
return fmt.Errorf("failed to render keycloak-config-cli defaults: %w", err)
@@ -304,8 +312,8 @@ func createKeycloakSecrets(ctx context.Context, client kubernetes.Interface, key
304312
Name: "nebari-realm-admin-credentials",
305313
Namespace: namespace,
306314
Labels: map[string]string{
307-
"app.kubernetes.io/part-of": NebariFoundationalPartOf,
308-
"app.kubernetes.io/managed-by": "nebari-infrastructure-core",
315+
labelPartOf: NebariFoundationalPartOf,
316+
labelManagedBy: labelManagedByValue,
309317
},
310318
},
311319
Type: corev1.SecretTypeOpaque,
@@ -325,8 +333,8 @@ func createKeycloakSecrets(ctx context.Context, client kubernetes.Interface, key
325333
Name: "argocd-oidc-client-secret",
326334
Namespace: namespace,
327335
Labels: map[string]string{
328-
"app.kubernetes.io/part-of": NebariFoundationalPartOf,
329-
"app.kubernetes.io/managed-by": "nebari-infrastructure-core",
336+
labelPartOf: NebariFoundationalPartOf,
337+
labelManagedBy: labelManagedByValue,
330338
},
331339
},
332340
Type: corev1.SecretTypeOpaque,
@@ -357,8 +365,8 @@ func createKeycloakImportSecret(ctx context.Context, client kubernetes.Interface
357365
Name: KeycloakImportSecretName,
358366
Namespace: KeycloakDefaultNamespace,
359367
Labels: map[string]string{
360-
"app.kubernetes.io/part-of": NebariFoundationalPartOf,
361-
"app.kubernetes.io/managed-by": "nebari-infrastructure-core",
368+
labelPartOf: NebariFoundationalPartOf,
369+
labelManagedBy: labelManagedByValue,
362370
},
363371
},
364372
Type: corev1.SecretTypeOpaque,
@@ -379,8 +387,8 @@ func createLandingPageSecrets(ctx context.Context, client kubernetes.Interface,
379387
Name: NebariLandingRedisSecretName,
380388
Namespace: namespace,
381389
Labels: map[string]string{
382-
"app.kubernetes.io/part-of": NebariFoundationalPartOf,
383-
"app.kubernetes.io/managed-by": "nebari-infrastructure-core",
390+
labelPartOf: NebariFoundationalPartOf,
391+
labelManagedBy: labelManagedByValue,
384392
},
385393
},
386394
Type: corev1.SecretTypeOpaque,

pkg/argocd/keycloak_defaults.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@ package argocd
22

33
import (
44
"bytes"
5+
"context"
56
_ "embed"
67
"fmt"
78
"text/template"
9+
10+
"go.opentelemetry.io/otel"
811
)
912

1013
// keycloakDefaultsRaw is the default keycloak-config-cli input for the nebari
@@ -30,13 +33,23 @@ const (
3033
// supplied TemplateData. Only the Domain field is substituted today (used
3134
// to build the argocd client's redirect URI); future fields can be wired
3235
// in as the schema grows.
33-
func RenderKeycloakDefaults(data TemplateData) ([]byte, error) {
34-
tmpl, err := template.New("keycloak_defaults.yaml").Parse(string(keycloakDefaultsRaw))
36+
//
37+
// missingkey=error makes a stray template field (e.g. a future {{ .Typo }})
38+
// fail loudly here rather than silently emitting "<no value>" into the realm
39+
// config, where it would surface only as a misconfigured Keycloak at runtime.
40+
func RenderKeycloakDefaults(ctx context.Context, data TemplateData) ([]byte, error) {
41+
tracer := otel.Tracer("nebari-infrastructure-core")
42+
_, span := tracer.Start(ctx, "argocd.RenderKeycloakDefaults")
43+
defer span.End()
44+
45+
tmpl, err := template.New("keycloak_defaults.yaml").Option("missingkey=error").Parse(string(keycloakDefaultsRaw))
3546
if err != nil {
47+
span.RecordError(err)
3648
return nil, fmt.Errorf("parse keycloak defaults template: %w", err)
3749
}
3850
var buf bytes.Buffer
3951
if err := tmpl.Execute(&buf, data); err != nil {
52+
span.RecordError(err)
4053
return nil, fmt.Errorf("render keycloak defaults: %w", err)
4154
}
4255
return buf.Bytes(), nil

pkg/argocd/keycloak_defaults.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
# Default keycloak-config-cli input for the nebari realm.
22
#
3-
# Phase 1a of the declarative Keycloak configuration approach (see
4-
# docs/superpowers/specs/2026-03-12-declarative-keycloak-config-design.md).
5-
# This file is embedded into the NIC binary via go:embed (see
3+
# Phase 1a of the declarative Keycloak configuration approach proposed in
4+
# nebari-infrastructure-core#154. This file is embedded into the NIC binary
5+
# via go:embed (see
66
# keycloak_defaults.go), rendered with TemplateData at deploy time, and
77
# written verbatim to the in-cluster Secret `keycloak-config-import`. The
88
# kcc Job mounts that Secret read-only and applies the file against

pkg/argocd/keycloak_defaults_test.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package argocd
22

33
import (
4+
"context"
45
"strings"
56
"testing"
67
)
@@ -11,7 +12,7 @@ import (
1112
// produce a valid Keycloak state; regressions here are silent (the YAML
1213
// renders fine but Keycloak gets misconfigured), so we check structure.
1314
func TestRenderKeycloakDefaults(t *testing.T) {
14-
rendered, err := RenderKeycloakDefaults(TemplateData{Domain: "test.example.com"})
15+
rendered, err := RenderKeycloakDefaults(context.Background(), TemplateData{Domain: "test.example.com"})
1516
if err != nil {
1617
t.Fatalf("RenderKeycloakDefaults error: %v", err)
1718
}
@@ -40,6 +41,21 @@ func TestRenderKeycloakDefaults(t *testing.T) {
4041
"clientId: argocd",
4142
"$(env:ARGOCD_CLIENT_SECRET)",
4243
"https://argocd.test.example.com/auth/callback",
44+
// Realm security posture — silent-but-damaging if a regression flips these.
45+
"sslRequired: external",
46+
"registrationAllowed: false",
47+
"bruteForceProtected: true",
48+
// Realm roles assigned to the bootstrap admin user.
49+
"- name: admin",
50+
"- name: user",
51+
// Group-membership mapper emits a multivalued groups claim.
52+
`multivalued: "true"`,
53+
// argocd client security flags + CORS origin (webOrigins is distinct
54+
// from redirectUris and Domain-templated, so assert it renders too).
55+
"publicClient: false",
56+
"standardFlowEnabled: true",
57+
"directAccessGrantsEnabled: false",
58+
"https://argocd.test.example.com\n",
4359
} {
4460
if !strings.Contains(output, want) {
4561
t.Errorf("expected %q in rendered realm config, got:\n%s", want, output)
@@ -62,4 +78,10 @@ func TestRenderKeycloakDefaults(t *testing.T) {
6278
if strings.Contains(output, "{{") || strings.Contains(output, "}}") {
6379
t.Errorf("rendered output contains unresolved template markers, got:\n%s", output)
6480
}
81+
82+
// missingkey=error should make a stray field error out, but guard against
83+
// a rendered "<no value>" leaking into the realm config regardless.
84+
if strings.Contains(output, "<no value>") {
85+
t.Errorf("rendered output contains \"<no value>\" from an unresolved field, got:\n%s", output)
86+
}
6587
}

0 commit comments

Comments
 (0)