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

Jump to bottom

Allow for not rendering a Kubernetes Secret #454

Open
reegnz opened this issue Jun 14, 2024 · 2 comments · May be fixed by #455
Open

Allow for not rendering a Kubernetes Secret #454

reegnz opened this issue Jun 14, 2024 · 2 comments · May be fixed by #455

Comments

@reegnz
Copy link

reegnz commented Jun 14, 2024 •
edited
Loading

At our company (and I believe other companies as well) we don't allow deploying raw Kubernetes Secrets in our CD systems with helm.

Looking at the helm chart I believe it could be restructured by splitting out the actual secrets (username and password) into kubernetes secrets, and refer to them as a mounted secret.

That way configs can be transparent, and it's also compatible with enterprise secret management practices where a separate secret handling mechanism is used to deploy secrets (be it sealedsecrets, external-secrets, vault CSI, or whatever else corporate policy dictates).

Either way I believe the rendering of Secrets should be made optional, but some configs could still be rendered.
So distinction of what's a secret and what isn't is essential in how the config is loaded.
@reegnz
Copy link
Author

reegnz commented Jun 14, 2024

AFAICT this could be used as a reference as to what is considered a secret:

policy-reporter/pkg/config/target_factory.go

Lines 803 to 906 in a21564a

switch c := config.(type) {
case *Loki:
if values.Host != "" {
c.Host = values.Host
}
if values.Username != "" {
c.Username = values.Username
}
if values.Password != "" {
c.Password = values.Password
}
case *Slack:
if values.Webhook != "" {
c.Webhook = values.Webhook
c.Channel = values.Channel
}
case *Discord:
if values.Webhook != "" {
c.Webhook = values.Webhook
}
case *Teams:
if values.Webhook != "" {
c.Webhook = values.Webhook
}
case *Elasticsearch:
if values.Host != "" {
c.Host = values.Host
}
if values.Username != "" {
c.Username = values.Username
}
if values.Password != "" {
c.Password = values.Password
}
if values.APIKey != "" {
c.APIKey = values.APIKey
}
if values.TypelessAPI != false {
c.TypelessAPI = values.TypelessAPI
}
case *S3:
if values.AccessKeyID != "" {
c.AccessKeyID = values.AccessKeyID
}
if values.SecretAccessKey != "" {
c.SecretAccessKey = values.SecretAccessKey
}
if values.KmsKeyID != "" {
c.KmsKeyID = values.KmsKeyID
}
case *Kinesis:
if values.AccessKeyID != "" {
c.AccessKeyID = values.AccessKeyID
}
if values.SecretAccessKey != "" {
c.SecretAccessKey = values.SecretAccessKey
}
case *SecurityHub:
if values.AccessKeyID != "" {
c.AccessKeyID = values.AccessKeyID
}
if values.SecretAccessKey != "" {
c.SecretAccessKey = values.SecretAccessKey
}
if values.AccountID != "" {
c.AccountID = values.AccessKeyID
}
case *GCS:
if values.Credentials != "" {
c.Credentials = values.Credentials
}
case *Webhook:
if values.Host != "" {
c.Host = values.Host
}
if values.Token != "" {
if c.Headers == nil {
c.Headers = make(map[string]string)
}
c.Headers["Authorization"] = values.Token
}
case *Telegram:
if values.Token != "" {
c.Token = values.Token
}
if values.Host != "" {
c.Host = values.Host
}
case *GoogleChat:
if values.Webhook != "" {
c.Webhook = values.Webhook
}
}
}

So only those fields should be rendered as kubernetes secrets (allowed to be provided from the outside instead of rendering it) and the rest should land in a configmap.

reegnz added a commit to reegnz/policy-reporter that referenced this issue Jun 14, 2024
@reegnz
Allow loading secrets from a separate config file
6340b67 
Currently all config is in a kubernetes secret because some of the
config keys the helm chart renders might be secrets.

This is a problem, as a lot of people don't want to, or don't have the
option to manage secrets with helm (eg. secret management is fully
decoupled from helm deployments).

To fix this, utilize go viper's solution that allows deep-merging
multiple config file contents. This allows for splitting out the
potentially sensitive keys into a separate config file, and then having
viper merge them back together.

The helm chart was modified so it retains backward compatibility with
the `existingTargetConfig` config option. If that key exists then it
is assumed that it is a kubernetes secret (as before), and the
separation of config values and secrets is not performed by the chart.

This PR should be able fixes kyverno#454
@reegnz reegnz linked a pull request Jun 14, 2024 that will close this issue
Draft
reegnz added a commit to reegnz/policy-reporter that referenced this issue Jun 14, 2024
@reegnz
Allow loading secrets from a separate config file
f9af63b 
Currently all config is in a kubernetes secret because some of the
config keys the helm chart renders might be secrets.

This is a problem, as a lot of people don't want to, or don't have the
option to manage secrets with helm (eg. secret management is fully
decoupled from helm deployments).

To fix this, utilize go viper's solution that allows deep-merging
multiple config file contents. This allows for splitting out the
potentially sensitive keys into a separate config file, and then having
viper merge them back together.

The helm chart was modified so it retains backward compatibility with
the `existingTargetConfig` config option. If that key exists then it
is assumed that it is a kubernetes secret (as before), and the
separation of config values and secrets is not performed by the chart.

This PR should be able fixes kyverno#454

Signed-off-by: Zoltán Reegn <zoltan.reegn@gmail.com>
@fjogeleit
Copy link
Member

Hey, very sorry for the late response, so would it be an simple solution for you to use the existing secretRef or mounttSecret functionality and decide if the config is applied as ConfigMap instead of secret? Your approach which multiple config filtes can have impact of users who using it without helm, and I would like to avoid breaking changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow loading secrets from a separate config file reegnz/policy-reporter
2 participants
@reegnz @fjogeleit