Skip to content

Commit

Permalink
Fix variable injection, add common labels to all resources, fix requi… (
Browse files Browse the repository at this point in the history
#256)

* Fix variable injection, add common labels to all resources, fix required options not being passed when etcd is enabled, fix apiservice management always required in clusterrole
Signed-off-by: Rafael da Fonseca <[email protected]>

Fix: allow env variable injection to work for database secrets
Fix: Add missing labels to pdb
Fix: apiservice management permissions
should always be required due to automatic migration code
Fix: some required command line options were not being added when using etcd as a database
Feat: Add commonLabels input variable, which allows adding arbitrary labels to all resources managed by the chart
  • Loading branch information
rsafonseca authored Feb 13, 2025
1 parent 59faa76 commit 8d4956b
Show file tree
Hide file tree
Showing 11 changed files with 77 additions and 27 deletions.
13 changes: 8 additions & 5 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ linters:
- bidichk
- bodyclose
- containedctx
- copyloopvar
- decorder
- dogsled
- durationcheck
- errcheck
- errname
- exportloopref
- gci
# - gochecknoinits
- gofmt
Expand All @@ -30,20 +30,23 @@ linters:
- nosprintfhostport
# - paralleltest
- staticcheck
- tenv
- thelper
- tparallel
- typecheck
- unconvert
- unused
- usetesting
- wastedassign
- whitespace

run:
timeout: 15m
skip-files:

issues:
exclude-files:
- ".+\\.generated.go"

output:
format: colored-line-number
sort-results: true
formats:
- format: colored-line-number
sort-results: true
3 changes: 2 additions & 1 deletion charts/reports-server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ helm install reports-server --namespace reports-server --create-namespace report
| serviceAccount.annotations | object | `{}` | Service account annotations |
| serviceAccount.name | string | `""` | Service account name (required if `serviceAccount.create` is `false`) |
| podAnnotations | object | `{}` | Pod annotations |
| commonLabels | object | `{}` | Labels to add to resources managed by the chart |
| podSecurityContext | object | `{"fsGroup":2000}` | Pod security context |
| podEnv | object | `{}` | Provide additional environment variables to the pods. Map with the same format as kubernetes deployment spec's env. |
| securityContext | object | See [values.yaml](values.yaml) | Container security context |
Expand Down Expand Up @@ -85,7 +86,7 @@ helm install reports-server --namespace reports-server --create-namespace report
| config.db.sslrootcert | string | `""` | Database SSL root cert |
| config.db.sslkey | string | `""` | Database SSL key |
| config.db.sslcert | string | `""` | Database SSL cert |
| apiServicesManagement.enabled | bool | `true` | Create a helm hooks delete api services on uninstall |
| apiServicesManagement.enabled | bool | `true` | Create a helm hooks to delete api services on uninstall |
| apiServicesManagement.installApiServices | object | `{"enabled":true,"installEphemeralReportsService":true}` | Install api services in manifest |
| apiServicesManagement.installApiServices.enabled | bool | `true` | Store reports in reports-server |
| apiServicesManagement.installApiServices.installEphemeralReportsService | bool | `true` | Store ephemeral reports in reports-server |
Expand Down
12 changes: 12 additions & 0 deletions charts/reports-server/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ Common labels
*/}}
{{- define "reports-server.labels" -}}
helm.sh/chart: {{ include "reports-server.chart" . }}
{{- if .Values.commonLabels }}
{{ include "reports-server.commonLabels" . }}
{{- end }}
{{ include "reports-server.selectorLabels" . }}
{{- if .Chart.AppVersion }}
app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
Expand All @@ -50,6 +53,15 @@ app.kubernetes.io/name: {{ include "reports-server.name" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
{{- end }}

{{/*
Common labels
*/}}
{{- define "reports-server.commonLabels" -}}
{{- with .Values.commonLabels }}
{{- toYaml . }}
{{- end }}
{{- end }}

{{/*
Create the name of the service account to use
*/}}
Expand Down
2 changes: 0 additions & 2 deletions charts/reports-server/templates/cluster-roles.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ rules:
- namespaces
verbs:
- get
{{- if .Values.apiServicesManagement.enabled }}
- apiGroups:
- apiregistration.k8s.io
resources:
Expand All @@ -47,7 +46,6 @@ rules:
resourceNames:
- v1.reports.kyverno.io
- v1alpha2.wgpolicyk8s.io
{{- end }}
- apiGroups:
- wgpolicyk8s.io
resources:
Expand Down
9 changes: 2 additions & 7 deletions charts/reports-server/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ spec:
{{- toYaml . | nindent 8 }}
{{- end }}
labels:
{{- include "reports-server.selectorLabels" . | nindent 8 }}
{{- include "reports-server.labels" . | nindent 8 }}
spec:
{{- with .Values.priorityClassName }}
priorityClassName: {{ . }}
Expand All @@ -45,20 +45,15 @@ spec:
{{- end }}
- --etcdEndpoints=https://etcd-0.etcd.{{ $.Release.Namespace }}:2379,https://etcd-1.etcd.{{ $.Release.Namespace }}:2379,https://etcd-2.etcd.{{ $.Release.Namespace }}:2379
{{- else }}
- --dbhost=$(DB_HOST)
- --dbport=$(DB_PORT)
- --dbuser=$(DB_USER)
- --dbpassword=$(DB_PASSWORD)
- --dbname=$(DB_DATABASE)
- --dbsslmode={{ .Values.config.db.sslmode }}
- --dbsslrootcert={{ .Values.config.db.sslrootcert }}
- --dbsslkey={{ .Values.config.db.sslkey }}
- --dbsslcert={{ .Values.config.db.sslcert }}
{{- end }}
- --servicename={{ include "reports-server.fullname" . }}
- --servicens={{ $.Release.Namespace }}
- --storereports={{ .Values.apiServicesManagement.installApiServices.enabled }}
- --storeephemeralreports={{ .Values.apiServicesManagement.installApiServices.installEphemeralReportsService }}
{{- end }}
- --cert-dir=/tmp
- --secure-port=4443
{{- if .Values.metrics.enabled }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ spec:
annotations:
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.apiServicesManagement.podLabels }}
labels:
{{- with .Values.apiServicesManagement.podLabels }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- include "reports-server.labels" . | nindent 8 }}
spec:
serviceAccount: {{ include "reports-server.serviceAccountName" . }}
{{- with .Values.apiServicesManagement.podSecurityContext }}
Expand Down
2 changes: 2 additions & 0 deletions charts/reports-server/templates/pod-disruption-budget.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ kind: PodDisruptionBudget
metadata:
name: {{ include "reports-server.fullname" . }}
namespace: {{ $.Release.Namespace }}
labels:
{{- include "reports-server.labels" . | nindent 4 }}
spec:
selector:
matchLabels:
Expand Down
5 changes: 4 additions & 1 deletion charts/reports-server/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ serviceAccount:
# -- Pod annotations
podAnnotations: {}

# -- Labels to add to resources managed by the chart
commonLabels: {}

# -- Pod security context
podSecurityContext:
fsGroup: 2000
Expand Down Expand Up @@ -227,7 +230,7 @@ config:
sslcert: ""

apiServicesManagement:
# -- Create a helm hooks delete api services on uninstall
# -- Create a helm hooks to delete api services on uninstall
enabled: true

# -- Install api services in manifest
Expand Down
13 changes: 13 additions & 0 deletions config/install-etcd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ kind: PodDisruptionBudget
metadata:
name: reports-server
namespace: reports-server
labels:
helm.sh/chart: reports-server-0.1.3
app.kubernetes.io/name: reports-server
app.kubernetes.io/instance: reports-server
app.kubernetes.io/version: "v0.1.3"
app.kubernetes.io/managed-by: Helm
spec:
selector:
matchLabels:
Expand Down Expand Up @@ -260,8 +266,11 @@ spec:
template:
metadata:
labels:
helm.sh/chart: reports-server-0.1.3
app.kubernetes.io/name: reports-server
app.kubernetes.io/instance: reports-server
app.kubernetes.io/version: "v0.1.3"
app.kubernetes.io/managed-by: Helm
spec:
priorityClassName: system-cluster-critical
serviceAccountName: reports-server
Expand All @@ -273,6 +282,10 @@ spec:
- --etcd
- --etcdSkipTLS
- --etcdEndpoints=https://etcd-0.etcd.reports-server:2379,https://etcd-1.etcd.reports-server:2379,https://etcd-2.etcd.reports-server:2379
- --servicename=reports-server
- --servicens=reports-server
- --storereports=true
- --storeephemeralreports=true
- --cert-dir=/tmp
- --secure-port=4443
- --authorization-always-allow-paths=/metrics
Expand Down
14 changes: 9 additions & 5 deletions config/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ kind: PodDisruptionBudget
metadata:
name: reports-server
namespace: reports-server
labels:
helm.sh/chart: reports-server-0.1.3
app.kubernetes.io/name: reports-server
app.kubernetes.io/instance: reports-server
app.kubernetes.io/version: "v0.1.3"
app.kubernetes.io/managed-by: Helm
spec:
selector:
matchLabels:
Expand Down Expand Up @@ -322,8 +328,11 @@ spec:
template:
metadata:
labels:
helm.sh/chart: reports-server-0.1.3
app.kubernetes.io/name: reports-server
app.kubernetes.io/instance: reports-server
app.kubernetes.io/version: "v0.1.3"
app.kubernetes.io/managed-by: Helm
spec:
priorityClassName: system-cluster-critical
serviceAccountName: reports-server
Expand All @@ -332,11 +341,6 @@ spec:
containers:
- name: reports-server
args:
- --dbhost=$(DB_HOST)
- --dbport=$(DB_PORT)
- --dbuser=$(DB_USER)
- --dbpassword=$(DB_PASSWORD)
- --dbname=$(DB_DATABASE)
- --dbsslmode=disable
- --dbsslrootcert=
- --dbsslkey=
Expand Down
28 changes: 23 additions & 5 deletions pkg/app/opts/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package opts
import (
"fmt"
"net"
"os"
"strconv"
"strings"

"github.com/kyverno/reports-server/pkg/api"
Expand Down Expand Up @@ -81,11 +83,6 @@ func (o *Options) Flags() (fs flag.NamedFlagSets) {
msfs.BoolVar(&o.EtcdConfig.Insecure, "etcdSkipTLS", true, "Skip TLS verification when connecting to etcd")
msfs.BoolVar(&o.ShowVersion, "version", false, "Show version")
msfs.StringVar(&o.Kubeconfig, "kubeconfig", o.Kubeconfig, "The path to the kubeconfig used to connect to the Kubernetes API server and the Kubelets (defaults to in-cluster config)")
msfs.StringVar(&o.DBHost, "dbhost", "reportsdb.kyverno", "Host url of postgres instance")
msfs.IntVar(&o.DBPort, "dbport", 5432, "Port of the postgres instance")
msfs.StringVar(&o.DBUser, "dbuser", "postgres", "Username to login into postgres")
msfs.StringVar(&o.DBPassword, "dbpassword", "password", "Password to login into postgres")
msfs.StringVar(&o.DBName, "dbname", "reportsdb", "Name of the database to store policy reports in")
msfs.StringVar(&o.DBSSLMode, "dbsslmode", "disable", "SSL mode of the postgres database.")
msfs.StringVar(&o.DBSSLRootCert, "dbsslrootcert", "", "Path to database root cert.")
msfs.StringVar(&o.DBSSLKey, "dbsslkey", "", "Path to database ssl key.")
Expand Down Expand Up @@ -126,6 +123,10 @@ func (o Options) ServerConfig() (*server.Config, error) {
if err != nil {
return nil, err
}
err = o.dbConfig()
if err != nil {
return nil, err
}

dbconfig := &db.PostgresConfig{
Host: o.DBHost,
Expand Down Expand Up @@ -214,3 +215,20 @@ func (o Options) restConfig() (*rest.Config, error) {
}
return config, nil
}

// dbConfig reads the database configuration directly from environment variables
// because these configurations contain sensitive data, this is not read directly from command line input,
// to enable usecases of env variable injection, such as using vault-env
func (o *Options) dbConfig() error {
o.DBHost = os.Getenv("DB_HOST")
o.DBName = os.Getenv("DB_DATABASE")
o.DBUser = os.Getenv("DB_USER")
o.DBPassword = os.Getenv("DB_PASSWORD")
dbPort, err := strconv.Atoi(os.Getenv("DB_PORT"))
if err != nil {
return err
} else {
o.DBPort = dbPort
}
return nil
}

0 comments on commit 8d4956b

Please sign in to comment.