Address Review Comments

Signed-off-by: Christian Kruse <[email protected]>

.circleci/config.yml

@@ -220,8 +220,8 @@ jobs:
           command: curl https://raw.githubusercontent.com/helm/helm/main/scripts/get-helm-3 | bash
       - run: make generate-skupper-deployment-cluster-scoped
       - run: make generate-skupper-deployment-namespace-scoped
-      - run: make generate-network-observer-minimal
-      - run: make generate-network-observer-openshift-tls
+      - run: make generate-network-observer-generic
+      - run: make generate-network-observer-openshift
       - run: mkdir skupper-setup
       - run: cp ./*.yaml skupper-setup
       - run:

Makefile

@@ -139,6 +139,12 @@ generate-network-observer-openshift:
 		--set prometheus.securityContext=null \
 		> skupper-network-observer-openshift.yaml
 
+generate-network-observer-devel:
+	helm template skupper-network-observer ./charts/network-observer/ \
+		--set auth.strategy=none \
+		--set extraArgs={"-cors-allow-all"} \
+		--set skipManagementLabels=true > skupper-network-observer-devel.yaml
+
 clean:
 	rm -rf skupper controller kube-adaptor \
 		network-observer generate-doc \

charts/network-observer/README.md

@@ -22,14 +22,46 @@ HTTP Basic authentication (username and password are `skupper`) and no ingress.
 
 ### Ingress
 
-Supports chosen service types (LoadBalancer, NodePort), Kubernetes Ingresses,
-and Openshift Routes. Defaults to a ClusterIP Service.
+By default the network-observer does not include an ingress. As a convenience,
+the chart contains options that can help expose the service externally.
+
+* Configure an ingress by setting `ingress.enabled=true` and setting appropriate
+values under `ingress`.
+
+Example values.yaml using the nginx ingress nginx controller with a
+user-provided TLS certificate
+```
+ingress:
+  enabled: true
+  className: "nginx"
+  annotations:
+    nginx.ingress.kubernetes.io/backend-protocol: "https"
+  hosts:
+    - host:  skupper-net-01.mycluster.local
+      paths:
+        - path: /
+          pathType: Prefix
+  tls:
+    - secretName: skupper-net-01-tls
+      hosts:
+        - skupper-net-01.mycluster.local
+```
+
+* Configure an openshift route by setting `route.enabled=true`.
+
+* Expose the service as type LoadBalancer `service.type=LoadBalancer`.
 
 ### TLS
 
 TLS is mandatory for this deployment. It can be configured as user provided, provided
 by openshift or by the skupper controller.
 
+To use an existing TLS secret, overwrite `tls.secretName`.
+
+To use an openshift generated service certificate, set
+`tls.openshiftIssued=true` and `tls.skupperIssued=false`. An annotation will be
+added to the service that should prompt openshift to provision a TLS secret.
+
 ### Authorization
 
 The network observer pod contains a reverse proxy that handles authorization
@@ -40,3 +72,18 @@ When authorization strategy is "openshift" an oauth2 proxy is used instead, and
 is configured to use the cluster identity provider for authorization. Openshift
 auth only works with ingress type Route.
 
+To set a secure basic auth credentials run:
+```
+# Use htpasswd to generate a new password file
+htpasswd -B -c passwords \
+    my-username;
+
+# Add a new secret with that password file
+kubectl create secret generic my-custom-auth \
+    --from-file=htpasswd=passwords;
+
+# Point the chart at the new secret
+helm install ... \
+    --set auth.basic.create=false \
+    --set auth.basic.secretName=my-custom-auth
+```

charts/network-observer/templates/NOTES.txt

@@ -0,0 +1,48 @@
+You have installed the skupper network observer!
+
+Accessing the console:
+{{- if .Values.ingress.enabled }}
+  The {{ include "network-observer.fullname" . }} service is exposed through an
+  Ingress and should be availalble external to the cluster at the following
+  locations.
+{{- range $host := .Values.ingress.hosts }}
+  {{- range .paths }}
+  http{{ if $.Values.ingress.tls }}s{{ end }}://{{ $host.host }}{{ .path }}
+  {{- end }}
+{{- end }}
+{{- else if .Values.route.enabled }}
+  The service is exposed through the {{ include "network-observer.fullname" . }} Route.
+  export ROUTE_HOST=$(kubectl get --namespace {{ .Release.Namespace }} -o jsonpath="{.spec.host}" route {{ include "network-observer.fullname" . }})
+  echo "https://$ROUTE_HOST"
+{{- else if contains "NodePort" .Values.service.type }}
+  export NODE_PORT=$(kubectl get --namespace {{ .Release.Namespace }} -o jsonpath="{.spec.ports[0].nodePort}" services {{ include "network-observer.fullname" . }})
+  export NODE_IP=$(kubectl get nodes --namespace {{ .Release.Namespace }} -o jsonpath="{.items[0].status.addresses[0].address}")
+  echo http://$NODE_IP:$NODE_PORT
+{{- else if contains "LoadBalancer" .Values.service.type }}
+     NOTE: It may take a few minutes for the LoadBalancer IP to be available.
+           You can watch its status by running 'kubectl get --namespace {{ .Release.Namespace }} svc -w {{ include "network-observer.fullname" . }}'
+  export SERVICE_IP=$(kubectl get svc --namespace {{ .Release.Namespace }} {{ include "network-observer.fullname" . }} --template "{{"{{ range (index .status.loadBalancer.ingress 0) }}{{.}}{{ end }}"}}")
+  echo http://$SERVICE_IP:{{ .Values.service.port }}
+{{- else if contains "ClusterIP" .Values.service.type }}
+  The network-observer application is exposed as a service inside of your
+  cluster. To access the application externally you must either enable an
+  ingress of some sort or use port forwarding to access the service
+  temporarily.
+  Expose the application at https://127.0.0.1:8443 with the command:
+  kubectl --namespace {{ .Release.Namespace }} port-forward service/{{ include "network-observer.fullname" . }} 8443:{{ .Values.service.port }}
+{{- end }}
+
+{{- if eq "basic" .Values.auth.strategy }}
+
+  Basic Authentication is enabled.
+
+  Users are configured in the {{ include "network-observer.basicAuthSecretName" . }} secret.
+  By default this chart includes placeholder credentials username="skupper" password="skupper".
+  This secret should be replaced to include user-provided credentials.
+
+{{- else if eq "openshift" .Values.auth.strategy }}
+
+  Openshift Authentication is enabled.
+  Users should be able to authenticate with the openshift cluster to access the console application.
+
+{{- end }}

charts/network-observer/templates/_deployment.yaml

@@ -1,4 +1,9 @@
+
 {{- define "network-observer.proxyVolumeMounts" -}}
+{{- $authStrategies := list "basic" "openshift" "none" }}
+{{- if not (has .Values.auth.strategy $authStrategies) }}
+  {{- fail (printf "auth.strategy must be one of %s" $authStrategies) }}
+{{- end }}
 {{- if not (eq .Values.auth.strategy "openshift") }}
 - name: nginx-config
   configMap:
@@ -16,6 +21,10 @@
 {{- define "network-observer.nginxProxySpec" -}}
 image: "{{ .Values.nginx.repository }}:{{ .Values.nginx.tag }}"
 imagePullPolicy: {{ .Values.nginx.pullPolicy }}
+{{- with .Values.nginx.securityContext }}
+securityContext:
+  {{- toYaml . | nindent 2 }}
+{{- end }}
 ports:
   - name: https
     containerPort: 8443
@@ -36,6 +45,10 @@ volumeMounts:
 {{- define "network-observer.openshiftOauthProxySpec" -}}
 image: "{{ .Values.openshiftOauthProxy.repository }}:{{ .Values.openshiftOauthProxy.tag }}"
 imagePullPolicy: {{ .Values.openshiftOauthProxy.pullPolicy }}
+{{- with .Values.openshiftOauthProxy.securityContext }}
+securityContext:
+  {{- toYaml . | nindent 2 }}
+{{- end }}
 args:
   - --https-address=:8443
   - --provider=openshift

charts/network-observer/templates/_helpers.tpl

@@ -75,12 +75,14 @@ Create the TLS Secret Name
 {{- define "network-observer.tlsSecretName" -}}
 {{- .Values.tls.secretName | default (printf "%s-tls" (include "network-observer.fullname" .)) }}
 {{- end }}
+
 {{/*
 Create the nginx configmap name
 */}}
 {{- define "network-observer.nginxConfigMapName" -}}
 {{- (printf "%s-nginx" (include "network-observer.fullname" .)) }}
 {{- end }}
+
 {{- define "network-observer.basicAuthSecretName" -}}
 {{- .Values.auth.basic.secretName | default (printf "%s-auth" (include "network-observer.fullname" .)) }}
 {{- end }}

charts/network-observer/templates/deployment.yaml

@@ -20,6 +20,10 @@ spec:
         {{- toYaml . | nindent 8 }}
         {{- end }}
     spec:
+      {{- with .Values.podSecurityContext }}
+      securityContext:
+        {{- toYaml . | nindent 8 }}
+      {{- end }}
       {{- with .Values.imagePullSecrets }}
       imagePullSecrets:
         {{- toYaml . | nindent 8 }}
@@ -32,6 +36,10 @@ spec:
         - name: network-observer
           image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
           imagePullPolicy: {{ .Values.image.pullPolicy }}
+          {{- with .Values.securityContext }}
+          securityContext:
+            {{- toYaml . | nindent 12 }}
+          {{- end }}
           args:
             - -listen=127.0.0.1:8080
             - -prometheus-api=http://127.0.0.1:9090
@@ -60,6 +68,10 @@ spec:
         - name: prometheus
           image: "{{ .Values.prometheus.repository }}:{{ .Values.prometheus.tag }}"
           imagePullPolicy: {{ .Values.prometheus.pullPolicy }}
+          {{- with .Values.prometheus.securityContext }}
+          securityContext:
+            {{- toYaml . | nindent 12 }}
+          {{- end }}
           args:
             - --config.file=/etc/prometheus/prometheus.yml
             - --storage.tsdb.path=/prometheus/
@@ -90,15 +102,3 @@ spec:
           defaultMode: 420
           secretName: {{ include "network-observer.tlsSecretName" . }}
       {{- (include "network-observer.proxyVolumeMounts" .) | nindent 6 }}
-      {{- with .Values.nodeSelector }}
-      nodeSelector:
-        {{- toYaml . | nindent 8 }}
-      {{- end }}
-      {{- with .Values.affinity }}
-      affinity:
-        {{- toYaml . | nindent 8 }}
-      {{- end }}
-      {{- with .Values.tolerations }}
-      tolerations:
-        {{- toYaml . | nindent 8 }}
-      {{- end }}

charts/network-observer/values.yaml

@@ -10,27 +10,42 @@ fullnameOverride: ""
 image:
   repository: quay.io/skupper/network-observer
   # This sets the pull policy for images.
-  pullPolicy: IfNotPresent
+  pullPolicy: Always
   # Overrides the image tag whose default is the chart appVersion.
   tag: ""
 
-# prometheus configures the promehteus container image
+# prometheus configures the prometheus container image
 prometheus:
-  repository: "docker.io/prom/prometheus"
+  repository: "quay.io/prometheus/prometheus"
   pullPolicy: IfNotPresent
   tag: "v3.0.1"
+  securityContext:
+    allowPrivilegeEscalation: false
+    capabilities:
+      drop:
+        - ALL
 
 # nginx configuration for reverse proxy (excluding openshift auth)
 nginx:
-  repository: "docker.io/nginxinc/nginx-unprivileged"
+  repository: "mirror.gcr.io/nginxinc/nginx-unprivileged"
   tag: "1.27.3-alpine"
   pullPolicy: IfNotPresent
+  securityContext:
+    allowPrivilegeEscalation: false
+    capabilities:
+      drop:
+        - ALL
 
 # openshift oauth proxy configuration when auth strategy is openshift
 openshiftOauthProxy:
   repository: "quay.io/openshift/origin-oauth-proxy"
   tag: "4.14.0"
   pullPolicy: IfNotPresent
+  securityContext:
+    allowPrivilegeEscalation: false
+    capabilities:
+      drop:
+        - ALL
 
 # extraArgs to pass to the network-observer container
 extraArgs:
@@ -116,10 +131,16 @@ resources: {}
   #   cpu: 100m
   #   memory: 128Mi
 
-nodeSelector: {}
-
-tolerations: []
-
-affinity: {}
+# pod level securityContext
+podSecurityContext:
+  seccompProfile:
+    type: RuntimeDefault
+
+# network-observer container securityContext
+securityContext:
+  allowPrivilegeEscalation: false
+  capabilities:
+    drop:
+      - ALL
 
 skipManagementLabels: false

cmd/network-observer/README.md

@@ -8,9 +8,8 @@ Console](https://github.com/skupperproject/skupper-console) web application.
 
 ## Deployment
 
-Deployment examples can be found in the [./resources](./resources/README.md)
-directory. These is still under development and should not be considered
-stable.
+The Network Observer can be deployed with the
+[network-observer](../../charts/network-observer/README.md) Helm Chart.
 
 ## API