From 9a8a78e433458aab4009929173655c82c10020dc Mon Sep 17 00:00:00 2001 From: Andrii Chubatiuk Date: Sat, 25 Jul 2026 10:47:30 +0300 Subject: [PATCH] helm-coverter: fixed VMUser conversion --- internal/converter/converter.go | 34 ++++++++++++++++------ internal/converter/converter_test.go | 42 +++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/internal/converter/converter.go b/internal/converter/converter.go index d1b52cd16..e14a6023f 100644 --- a/internal/converter/converter.go +++ b/internal/converter/converter.go @@ -104,6 +104,7 @@ type VMAuthConfigValues struct { // operator types whose field names already match vmauth's native keys. Backend TLS // (tls_ca_file etc.) isn't covered: it doesn't map onto the operator's nested tlsConfig. type VMAuthConfigUser struct { + Name string `yaml:"name,omitempty" json:"name,omitempty"` Username string `yaml:"username,omitempty" json:"username,omitempty"` Password string `yaml:"password,omitempty" json:"password,omitempty"` BearerToken string `yaml:"bearer_token,omitempty" json:"bearer_token,omitempty"` @@ -1772,24 +1773,41 @@ func ConvertVMAuthUsers(vmauthName, namespace string, values *VMAuthHelmValues) } users := make([]*vmv1beta1.VMUser, 0, len(values.Config.Users)) for i, u := range values.Config.Users { - if u.Username == "" { - return nil, fmt.Errorf("config.users[%d]: username is required", i) + if u.Username == "" && u.BearerToken == "" { + return nil, fmt.Errorf("config.users[%d]: one of username or bearer_token is required", i) } + objectName := u.Name + if objectName == "" { + objectName = u.Username + } + if objectName == "" { + objectName = fmt.Sprintf("user-%d", i) + } + targetRefs, err := convertVMAuthConfigUserTargetRefs(u) if err != nil { - return nil, fmt.Errorf("config.users[%d] (username=%q): %w", i, u.Username, err) + return nil, fmt.Errorf("config.users[%d] (name=%q): %w", i, objectName, err) } spec := vmv1beta1.VMUserSpec{ - Username: &u.Username, TargetRefs: targetRefs, MetricLabels: u.MetricLabels, VMUserConfigOptions: u.VMUserConfigOptions, } - if u.Password != "" { - spec.Password = &u.Password - } + // username and bearerToken can't both be set; keep username as the display Name. + displayName := u.Name if u.BearerToken != "" { spec.BearerToken = &u.BearerToken + if displayName == "" { + displayName = u.Username + } + } else { + spec.Username = &u.Username + if u.Password != "" { + spec.Password = &u.Password + } + } + if displayName != "" { + spec.Name = &displayName } users = append(users, &vmv1beta1.VMUser{ TypeMeta: metav1.TypeMeta{ @@ -1797,7 +1815,7 @@ func ConvertVMAuthUsers(vmauthName, namespace string, values *VMAuthHelmValues) Kind: "VMUser", }, ObjectMeta: metav1.ObjectMeta{ - Name: sanitizeK8sName(u.Username), + Name: sanitizeK8sName(objectName), Namespace: namespace, Labels: vmAuthUserSelectorLabels(vmauthName), }, diff --git a/internal/converter/converter_test.go b/internal/converter/converter_test.go index c5ba667c0..01fca221c 100644 --- a/internal/converter/converter_test.go +++ b/internal/converter/converter_test.go @@ -1512,6 +1512,9 @@ config: assert.Equal(t, "loadbalanced-user", second.Name) require.NotNil(t, second.Spec.BearerToken) assert.Equal(t, "mytoken", *second.Spec.BearerToken) + assert.Nil(t, second.Spec.Username, "username must not be set alongside bearerToken, vmauth itself rejects that combination") + require.NotNil(t, second.Spec.Name) + assert.Equal(t, "LoadBalanced_User", *second.Spec.Name, "username is preserved as the display Name when bearer_token is used") require.Len(t, second.Spec.TargetRefs, 1) assert.Equal(t, []string{"/api/v1/query.*"}, second.Spec.TargetRefs[0].Paths) require.NotNil(t, second.Spec.TargetRefs[0].Static) @@ -1522,7 +1525,7 @@ func TestConvertVMAuthUsers_Errors(t *testing.T) { _, err := ConvertVMAuthUsers("test-name", "test-ns", &VMAuthHelmValues{ Config: &VMAuthConfigValues{Users: []VMAuthConfigUser{{Password: "x"}}}, }) - assert.ErrorContains(t, err, "username is required") + assert.ErrorContains(t, err, "one of username or bearer_token is required") _, err = ConvertVMAuthUsers("test-name", "test-ns", &VMAuthHelmValues{ Config: &VMAuthConfigValues{Users: []VMAuthConfigUser{{Username: "foo"}}}, @@ -1530,6 +1533,43 @@ func TestConvertVMAuthUsers_Errors(t *testing.T) { assert.ErrorContains(t, err, "url_prefix is required") } +// TestConvertVMAuthUsers_UsernameAndBearerToken reproduces #2441. +func TestConvertVMAuthUsers_UsernameAndBearerToken(t *testing.T) { + users, err := ConvertVMAuthUsers("test-name", "test-ns", &VMAuthHelmValues{ + Config: &VMAuthConfigValues{Users: []VMAuthConfigUser{{ + Username: "svc-account", + BearerToken: "mytoken", + URLPrefix: vmv1beta1.StringOrArray{"http://vmselect:8481/"}, + }}}, + }) + require.NoError(t, err) + require.Len(t, users, 1) + u := users[0] + assert.Nil(t, u.Spec.Username) + require.NotNil(t, u.Spec.BearerToken) + assert.Equal(t, "mytoken", *u.Spec.BearerToken) + require.NotNil(t, u.Spec.Name) + assert.Equal(t, "svc-account", *u.Spec.Name) + assert.Equal(t, "svc-account", u.Name, "object name still derives from username when name isn't explicitly set") +} + +// TestConvertVMAuthUsers_ExplicitName covers a user config that sets name distinctly from username. +func TestConvertVMAuthUsers_ExplicitName(t *testing.T) { + users, err := ConvertVMAuthUsers("test-name", "test-ns", &VMAuthHelmValues{ + Config: &VMAuthConfigValues{Users: []VMAuthConfigUser{{ + Name: "readable-label", + BearerToken: "mytoken", + URLPrefix: vmv1beta1.StringOrArray{"http://vmselect:8481/"}, + }}}, + }) + require.NoError(t, err) + require.Len(t, users, 1) + u := users[0] + require.NotNil(t, u.Spec.Name) + assert.Equal(t, "readable-label", *u.Spec.Name) + assert.Equal(t, "readable-label", u.Name) +} + func TestConvertVMAuthUsers_NoConfig(t *testing.T) { users, err := ConvertVMAuthUsers("test-name", "test-ns", &VMAuthHelmValues{}) require.NoError(t, err)