Skip to content

Commit

Permalink
controllers/vmalert: moves remote secret into k8s Secret
Browse files Browse the repository at this point in the history 
linked secrets will be mounted into pod and do not exposed in plain text as container args.
it improves security
#440
  • Loading branch information
@f41gh7
f41gh7 committed Dec 29, 2022
1 parent 18df7aa commit 857910e
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 45 deletions.
4 changes: 0 additions & 4 deletions controllers/factory/scrapes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ func CreateOrUpdateConfigurationSecret(ctx context.Context, cr *victoriametricsv
return nil, fmt.Errorf("loading additional scrape configs from Secret failed: %w", err)
}

// how to store remoteWrite secrets properly?
// it must be keyed by some value
// e.g. basicAuth/idx/passwordFile
// e.g. bearer/idx/tokenFile
// Update secret based on the most recent configuration.
generatedConfig, err := generateConfig(
cr,
Expand Down
151 changes: 111 additions & 40 deletions controllers/factory/vmalert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,25 @@ import (
)

const (
vmAlertConfigDir = "/etc/vmalert/config"
vmAlertConfigDir = "/etc/vmalert/config"
datasourceKey = "datasource"
remoteReadKey = "remoteRead"
remoteWriteKey = "remoteWrite"
notifierConfigMountPath = `/etc/vm/notifier_config`
vmalertConfigSecretsDir = "/etc/vmalert/remote_secrets"
bearerTokenKey = "bearerToken"
basicAuthPasswordKey = "basicAuthPassword"
oauth2SecretKey = "oauth2SecretKey"
)

func buildNotifierKey(idx int) string {
return fmt.Sprintf("notifier-%d", idx)
}

func buildRemoteSecretKey(source, suffix string) string {
return fmt.Sprintf("%s_%s", strings.ToUpper(source), strings.ToUpper(suffix))
}

func CreateOrUpdateVMAlertService(ctx context.Context, cr *victoriametricsv1beta1.VMAlert, rclient client.Client, c *config.BaseOperatorConf) (*corev1.Service, error) {
if cr.Spec.Port == "" {
cr.Spec.Port = c.VMAlertDefault.Port
Expand Down Expand Up @@ -57,6 +73,56 @@ func CreateOrUpdateVMAlertService(ctx context.Context, cr *victoriametricsv1beta
return reconcileServiceForCRD(ctx, rclient, newService)
}

func createOrUpdateVMAlertSecret(ctx context.Context, rclient client.Client, cr *victoriametricsv1beta1.VMAlert, ssCache map[string]*authSecret, c *config.BaseOperatorConf) error {
s := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: cr.PrefixedName(),
Annotations: cr.AnnotationsFiltered(),
Labels: c.Labels.Merge(cr.AllLabels()),
Namespace: cr.Namespace,
OwnerReferences: cr.AsOwner(),
},
Data: map[string][]byte{},
}
addSecretKeys := func(sourcePrefix string, ha victoriametricsv1beta1.HTTPAuth) {
ba := ssCache[sourcePrefix]
if ba == nil {
return
}
if ha.BasicAuth != nil && ba.BasicAuthCredentials != nil {
if len(ba.password) > 0 {
s.Data[buildRemoteSecretKey(sourcePrefix, basicAuthPasswordKey)] = []byte(ba.password)
}

}
if ha.BearerAuth != nil && len(ba.bearerValue) > 0 {
s.Data[buildRemoteSecretKey(sourcePrefix, bearerTokenKey)] = []byte(ba.bearerValue)
}
if ha.OAuth2 != nil && ba.oauthCreds != nil {
if len(ba.clientSecret) > 0 {
s.Data[buildRemoteSecretKey(sourcePrefix, oauth2SecretKey)] = []byte(ba.clientSecret)
}
}
}
if cr.Spec.RemoteRead != nil {
addSecretKeys(remoteReadKey, cr.Spec.RemoteRead.HTTPAuth)
}
if cr.Spec.RemoteWrite != nil {
addSecretKeys(remoteWriteKey, cr.Spec.RemoteWrite.HTTPAuth)
}
addSecretKeys(datasourceKey, cr.Spec.Datasource.HTTPAuth)
for idx, nf := range cr.Spec.Notifiers {
addSecretKeys(buildNotifierKey(idx), nf.HTTPAuth)
}
if err := rclient.Create(ctx, s); err != nil {
if errors.IsAlreadyExists(err) {
return rclient.Update(ctx, s)
}
return fmt.Errorf("cannot create secret for vmalert remote secrets: %w", err)
}
return nil
}

func CreateOrUpdateVMAlert(ctx context.Context, cr *victoriametricsv1beta1.VMAlert, rclient client.Client, c *config.BaseOperatorConf, cmNames []string) (reconcile.Result, error) {
l := log.WithValues("controller", "vmalert.crud", "vmalert", cr.Name)
// copy to avoid side effects.
Expand Down Expand Up @@ -115,7 +181,10 @@ func CreateOrUpdateVMAlert(ctx context.Context, cr *victoriametricsv1beta1.VMAle
}
remoteSecrets, err := loadVMAlertRemoteSecrets(ctx, rclient, cr)
if err != nil {
l.Error(err, "cannot get basic auth secretsInNs for vmalert")
return reconcile.Result{}, err
}
// create secret for remoteSecrets
if err := createOrUpdateVMAlertSecret(ctx, rclient, cr, remoteSecrets, c); err != nil {
return reconcile.Result{}, err
}

Expand Down Expand Up @@ -219,7 +288,16 @@ func vmAlertSpecGen(cr *victoriametricsv1beta1.VMAlert, c *config.BaseOperatorCo
SecretName: cr.TLSAssetName(),
},
},
})
},
corev1.Volume{
Name: "remote-secrets",
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: cr.PrefixedName(),
},
},
},
)

for _, name := range ruleConfigMapNames {
volumes = append(volumes, corev1.Volume{
Expand All @@ -241,6 +319,11 @@ func vmAlertSpecGen(cr *victoriametricsv1beta1.VMAlert, c *config.BaseOperatorCo
ReadOnly: true,
MountPath: tlsAssetsDir,
},
corev1.VolumeMount{
Name: "remote-secrets",
ReadOnly: true,
MountPath: vmalertConfigSecretsDir,
},
)
if cr.Spec.NotifierConfigRef != nil {
volumes = append(volumes, corev1.Volume{
Expand Down Expand Up @@ -428,15 +511,15 @@ func buildVMAlertAuthArgs(args []string, flagPrefix string, ha victoriametricsv1
if ha.BasicAuth != nil {
args = append(args, fmt.Sprintf("-%s.basicAuth.username=%s", flagPrefix, s.username))
if len(s.password) > 0 {
args = append(args, fmt.Sprintf("-%s.basicAuth.password=%s", flagPrefix, s.password))
args = append(args, fmt.Sprintf("-%s.basicAuth.passwordFile=%s", flagPrefix, path.Join(vmalertConfigSecretsDir, buildRemoteSecretKey(flagPrefix, basicAuthPasswordKey))))
}
if len(ha.BasicAuth.PasswordFile) > 0 {
args = append(args, fmt.Sprintf("-%s.basicAuth.passwordFile=%s", flagPrefix, ha.BasicAuth.PasswordFile))
}
}
if ha.BearerAuth != nil {
if len(s.bearerValue) > 0 {
args = append(args, fmt.Sprintf("-%s.bearerToken=%s", flagPrefix, s.bearerValue))
args = append(args, fmt.Sprintf("-%s.bearerTokenFile=%s", flagPrefix, path.Join(vmalertConfigSecretsDir, buildRemoteSecretKey(flagPrefix, bearerTokenKey))))
} else if len(ha.BearerAuth.TokenFilePath) > 0 {
args = append(args, fmt.Sprintf("-%s.bearerTokenFile=%s", flagPrefix, ha.BearerAuth.TokenFilePath))
}
Expand All @@ -445,7 +528,7 @@ func buildVMAlertAuthArgs(args []string, flagPrefix string, ha victoriametricsv1
if len(ha.OAuth2.ClientSecretFile) > 0 {
args = append(args, fmt.Sprintf("-%s.oauth2.clientSecretFile=%s", flagPrefix, ha.OAuth2.ClientSecretFile))
} else {
args = append(args, fmt.Sprintf("-%s.oauth2.clientSecret=%s", flagPrefix, s.oauthCreds.clientSecret))
args = append(args, fmt.Sprintf("-%s.oauth2.clientSecretFile=%s", flagPrefix, path.Join(vmalertConfigSecretsDir, buildRemoteSecretKey(flagPrefix, oauth2SecretKey))))
}
args = append(args, fmt.Sprintf("-%s.oauth2.clientID=%s", flagPrefix, s.oauthCreds.clientID))
args = append(args, fmt.Sprintf("-%s.oauth2.tokenUrl=%s", flagPrefix, ha.OAuth2.TokenURL))
Expand All @@ -456,25 +539,23 @@ func buildVMAlertAuthArgs(args []string, flagPrefix string, ha victoriametricsv1
return args
}

// todo add oauth2
func buildVMAlertArgs(cr *victoriametricsv1beta1.VMAlert, ruleConfigMapNames []string, remoteSecrets map[string]*authSecret) []string {
args := []string{
fmt.Sprintf("-datasource.url=%s", cr.Spec.Datasource.URL),
}

args = buildHeadersArg("datasource.headers", args, cr.Spec.Datasource.HTTPAuth.Headers)
args = append(args, BuildNotifiersArgs(cr, remoteSecrets)...)

args = buildVMAlertAuthArgs(args, "datasource", cr.Spec.Datasource.HTTPAuth, remoteSecrets)
args = buildVMAlertAuthArgs(args, datasourceKey, cr.Spec.Datasource.HTTPAuth, remoteSecrets)

if cr.Spec.Datasource.HTTPAuth.TLSConfig != nil {
tlsConf := cr.Spec.Datasource.HTTPAuth.TLSConfig
args = tlsConf.AsArgs(args, "datasource", cr.Namespace)
args = tlsConf.AsArgs(args, datasourceKey, cr.Namespace)
}

if cr.Spec.RemoteWrite != nil {
args = append(args, fmt.Sprintf("-remoteWrite.url=%s", cr.Spec.RemoteWrite.URL))
args = buildVMAlertAuthArgs(args, "remoteWrite", cr.Spec.RemoteWrite.HTTPAuth, remoteSecrets)
args = buildVMAlertAuthArgs(args, remoteWriteKey, cr.Spec.RemoteWrite.HTTPAuth, remoteSecrets)
args = buildHeadersArg("remoteWrite.headers", args, cr.Spec.RemoteWrite.HTTPAuth.Headers)
if cr.Spec.RemoteWrite.Concurrency != nil {
args = append(args, fmt.Sprintf("-remoteWrite.concurrency=%d", *cr.Spec.RemoteWrite.Concurrency))
Expand All @@ -490,7 +571,7 @@ func buildVMAlertArgs(cr *victoriametricsv1beta1.VMAlert, ruleConfigMapNames []s
}
if cr.Spec.RemoteWrite.HTTPAuth.TLSConfig != nil {
tlsConf := cr.Spec.RemoteWrite.HTTPAuth.TLSConfig
args = tlsConf.AsArgs(args, "remoteWrite", cr.Namespace)
args = tlsConf.AsArgs(args, remoteWriteKey, cr.Namespace)
}
}
for k, v := range cr.Spec.ExternalLabels {
Expand All @@ -499,14 +580,14 @@ func buildVMAlertArgs(cr *victoriametricsv1beta1.VMAlert, ruleConfigMapNames []s

if cr.Spec.RemoteRead != nil {
args = append(args, fmt.Sprintf("-remoteRead.url=%s", cr.Spec.RemoteRead.URL))
args = buildVMAlertAuthArgs(args, "remoteRead", cr.Spec.RemoteRead.HTTPAuth, remoteSecrets)
args = buildVMAlertAuthArgs(args, remoteReadKey, cr.Spec.RemoteRead.HTTPAuth, remoteSecrets)
args = buildHeadersArg("remoteRead.headers", args, cr.Spec.RemoteRead.HTTPAuth.Headers)
if cr.Spec.RemoteRead.Lookback != nil {
args = append(args, fmt.Sprintf("-remoteRead.lookback=%s", *cr.Spec.RemoteRead.Lookback))
}
if cr.Spec.RemoteRead.HTTPAuth.TLSConfig != nil {
tlsConf := cr.Spec.RemoteRead.HTTPAuth.TLSConfig
args = tlsConf.AsArgs(args, "remoteRead", cr.Namespace)
args = tlsConf.AsArgs(args, remoteReadKey, cr.Namespace)
}

}
Expand Down Expand Up @@ -582,35 +663,34 @@ func loadVMAlertRemoteSecrets(
return &as, nil
}
for i, notifier := range cr.Spec.Notifiers {
key := cr.NotifierAsMapKey(i)
as, err := loadHTTPAuthSecrets(ctx, rclient, ns, notifier.HTTPAuth)
if err != nil {
return nil, err
}
authSecretsBySource[key] = as
authSecretsBySource[cr.NotifierAsMapKey(i)] = as
}
// load basic auth for datasource configuration
as, err := loadHTTPAuthSecrets(ctx, rclient, ns, datasource.HTTPAuth)
if err != nil {
return nil, fmt.Errorf("could not generate basicAuth for datasource config. %w", err)
}
authSecretsBySource["datasource"] = as
authSecretsBySource[datasourceKey] = as

// load basic auth for remote write configuration
if remoteWrite != nil {
as, err := loadHTTPAuthSecrets(ctx, rclient, ns, remoteWrite.HTTPAuth)
if err != nil {
return nil, fmt.Errorf("could not generate basicAuth for datasource config. %w", err)
}
authSecretsBySource["remoteWrite"] = as
authSecretsBySource[remoteWriteKey] = as
}
// load basic auth for remote write configuration
if remoteRead != nil {
as, err := loadHTTPAuthSecrets(ctx, rclient, ns, remoteRead.HTTPAuth)
if err != nil {
return nil, fmt.Errorf("could not generate basicAuth for datasource config. %w", err)
}
authSecretsBySource["remoteRead"] = as
authSecretsBySource[remoteReadKey] = as
}
return authSecretsBySource, nil
}
Expand Down Expand Up @@ -737,8 +817,6 @@ func loadTLSAssetsForVMAlert(ctx context.Context, rclient client.Client, cr *vic
return assets, nil
}

const notifierConfigMountPath = `/etc/vm/notifier_config`

func BuildNotifiersArgs(cr *victoriametricsv1beta1.VMAlert, ntBasicAuth map[string]*authSecret) []string {
var finalArgs []string
var notifierArgs []remoteFlag
Expand All @@ -750,7 +828,6 @@ func BuildNotifiersArgs(cr *victoriametricsv1beta1.VMAlert, ntBasicAuth map[stri

url := remoteFlag{flagSetting: "-notifier.url=", isNotNull: true}
authUser := remoteFlag{flagSetting: "-notifier.basicAuth.username="}
authPassword := remoteFlag{flagSetting: "-notifier.basicAuth.password="}
authPasswordFile := remoteFlag{flagSetting: "-notifier.basicAuth.passwordFile="}
tlsCAs := remoteFlag{flagSetting: "-notifier.tlsCAFile="}
tlsCerts := remoteFlag{flagSetting: "-notifier.tlsCertFile="}
Expand All @@ -759,8 +836,6 @@ func BuildNotifiersArgs(cr *victoriametricsv1beta1.VMAlert, ntBasicAuth map[stri
tlsInSecure := remoteFlag{flagSetting: "-notifier.tlsInsecureSkipVerify="}
headers := remoteFlag{flagSetting: "-notifier.headers="}
bearerTokenPath := remoteFlag{flagSetting: "-notifier.bearerTokenFile="}
bearerToken := remoteFlag{flagSetting: "-notifier.bearerToken="}
oauth2Secret := remoteFlag{flagSetting: "-notifier.oauth2.clientSecret="}
oauth2SecretFile := remoteFlag{flagSetting: "-notifier.oauth2.clientSecretFile="}
oauth2ClientID := remoteFlag{flagSetting: "-notifier.oauth2.clientID="}
oauth2Scopes := remoteFlag{flagSetting: "-notifier.oauth2.scopes="}
Expand Down Expand Up @@ -823,39 +898,36 @@ func BuildNotifiersArgs(cr *victoriametricsv1beta1.VMAlert, ntBasicAuth map[stri
}
headerFlagValue = strings.TrimSuffix(headerFlagValue, "^^")
headers.flagSetting += fmt.Sprintf("%s,", headerFlagValue)
var user, pass, passFile string
var user, passFile string
s := ntBasicAuth[cr.NotifierAsMapKey(i)]
if nt.HTTPAuth.BasicAuth != nil {
authUser.isNotNull = true
user = s.username
if len(s.password) > 0 {
authPassword.isNotNull = true
pass = s.password
passFile = path.Join(vmalertConfigSecretsDir, buildRemoteSecretKey(buildNotifierKey(i), basicAuthPasswordKey))
authPasswordFile.isNotNull = true
}
if len(nt.HTTPAuth.BasicAuth.PasswordFile) > 0 {
passFile = nt.HTTPAuth.BasicAuth.PasswordFile
authPasswordFile.isNotNull = true
}
}
authUser.flagSetting += fmt.Sprintf("\"%s\",", strings.ReplaceAll(user, `"`, `\"`))
authPassword.flagSetting += fmt.Sprintf("\"%s\",", strings.ReplaceAll(pass, `"`, `\"`))
authPasswordFile.flagSetting += fmt.Sprintf("%s,", passFile)

var tokenPath, tokenValue string
var tokenPath string
if nt.HTTPAuth.BearerAuth != nil {
// todo implement token secret
if len(nt.HTTPAuth.BearerAuth.TokenFilePath) > 0 {
bearerTokenPath.isNotNull = true
tokenPath = nt.HTTPAuth.BearerAuth.TokenFilePath
} else if len(s.bearerValue) > 0 {
bearerToken.isNotNull = true
tokenValue = s.bearerValue
bearerTokenPath.isNotNull = true
tokenPath = path.Join(vmalertConfigSecretsDir, buildRemoteSecretKey(buildNotifierKey(i), bearerTokenKey))
}
}
bearerTokenPath.flagSetting += fmt.Sprintf("%s,", tokenPath)
bearerToken.flagSetting += fmt.Sprintf("%s,", tokenValue)
// todo implement oauth2
var scopes, tokenURL, secretValue, secretFile, clientID string
var scopes, tokenURL, secretFile, clientID string
if nt.OAuth2 != nil {
if len(nt.OAuth2.Scopes) > 0 {
oauth2Scopes.isNotNull = true
Expand All @@ -868,8 +940,8 @@ func BuildNotifiersArgs(cr *victoriametricsv1beta1.VMAlert, ntBasicAuth map[stri
clientID = s.clientID
oauth2ClientID.isNotNull = true
if len(s.clientSecret) > 0 {
oauth2Secret.isNotNull = true
secretValue = s.clientSecret
oauth2SecretFile.isNotNull = true
secretFile = path.Join(vmalertConfigSecretsDir, buildRemoteSecretKey(buildNotifierKey(i), oauth2SecretKey))
} else {
oauth2SecretFile.isNotNull = true
secretFile = nt.OAuth2.ClientSecretFile
Expand All @@ -878,12 +950,11 @@ func BuildNotifiersArgs(cr *victoriametricsv1beta1.VMAlert, ntBasicAuth map[stri
oauth2Scopes.flagSetting += fmt.Sprintf("%s,", scopes)
oauth2TokenURL.flagSetting += fmt.Sprintf("%s,", tokenURL)
oauth2ClientID.flagSetting += fmt.Sprintf("%s,", clientID)
oauth2Secret.flagSetting += fmt.Sprintf("%s,", secretValue)
oauth2SecretFile.flagSetting += fmt.Sprintf("%s,", secretFile)
}
notifierArgs = append(notifierArgs, url, authUser, authPassword)
notifierArgs = append(notifierArgs, tlsServerName, tlsKeys, tlsCerts, tlsCAs, tlsInSecure, headers, bearerTokenPath, authPasswordFile, bearerToken)
notifierArgs = append(notifierArgs, oauth2SecretFile, oauth2ClientID, oauth2Scopes, oauth2Secret, oauth2TokenURL)
notifierArgs = append(notifierArgs, url, authUser, authPasswordFile)
notifierArgs = append(notifierArgs, tlsServerName, tlsKeys, tlsCerts, tlsCAs, tlsInSecure, headers, bearerTokenPath)
notifierArgs = append(notifierArgs, oauth2SecretFile, oauth2ClientID, oauth2Scopes, oauth2TokenURL)

for _, remoteArgType := range notifierArgs {
if remoteArgType.isNotNull {
Expand Down
2 changes: 1 addition & 1 deletion controllers/factory/vmalert_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ func TestBuildNotifiers(t *testing.T) {
},
ntBasicAuth: map[string]*authSecret{"vmalert///0": &authSecret{oauthCreds: &oauthCreds{clientSecret: "some-secret", clientID: "some-id"}}, "vmalert///1": &authSecret{bearerValue: "some-v"}},
},
want: []string{"-notifier.url=http://1,http://2", "-notifier.headers=key=value^^key2=value2,key3=value3^^key4=value4", "-notifier.bearerToken=,some-v", "-notifier.oauth2.clientID=some-id,", "-notifier.oauth2.scopes=1,2,", "-notifier.oauth2.clientSecret=some-secret,", "-notifier.oauth2.tokenUrl=http://some-url,"},
want: []string{"-notifier.url=http://1,http://2", "-notifier.headers=key=value^^key2=value2,key3=value3^^key4=value4", "-notifier.bearerTokenFile=,/etc/vmalert/remote_secrets/NOTIFIER-1_BEARERTOKEN", "-notifier.oauth2.clientSecretFile=/etc/vmalert/remote_secrets/NOTIFIER-0_OAUTH2SECRETKEY,", "-notifier.oauth2.clientID=some-id,", "-notifier.oauth2.scopes=1,2,", "-notifier.oauth2.tokenUrl=http://some-url,"},
},
}
for _, tt := range tests {
Expand Down

0 comments on commit 857910e

Please sign in to comment.