From f311f7f3e3df23cf8edf8efc31076c916b05d5a1 Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Wed, 18 Dec 2024 09:51:24 +0100 Subject: [PATCH 01/12] ROX-27129: remove tenant-resources helm chart --- .openshift-ci/tests/e2e.sh | 3 - .openshift-ci/tests/netpol-test.sh | 171 ------------- fleetshard/pkg/central/charts/charts.go | 11 +- fleetshard/pkg/central/charts/charts_test.go | 6 - .../charts/data/tenant-resources/Chart.yaml | 6 - .../charts/data/tenant-resources/README.md | 3 - .../config/default-annotations.yaml.tpl | 0 .../config/default-labels.yaml.tpl | 4 - .../local-network-cidr-ranges-ipv6.yaml.tpl | 2 - .../config/local-network-cidr-ranges.yaml.tpl | 6 - .../tenant-resources/templates/_helpers.tpl | 19 -- .../templates/network-policy-central.yaml | 112 --------- .../templates/network-policy-default.yaml | 37 --- .../templates/network-policy-scanner-v4.yaml | 162 ------------- .../templates/network-policy-scanner.yaml | 83 ------- .../templates/vpa-central.yaml | 22 -- .../charts/data/tenant-resources/values.yaml | 7 - .../pkg/central/reconciler/reconciler.go | 18 +- .../pkg/central/reconciler/reconciler_test.go | 229 ------------------ .../reconciler/tenant_chart_reconciler.go | 212 ---------------- .../tenant_chart_reconciler_test.go | 62 ----- .../pkg/central/reconciler/tenant_cleanup.go | 26 +- 22 files changed, 11 insertions(+), 1190 deletions(-) delete mode 100755 .openshift-ci/tests/netpol-test.sh delete mode 100644 fleetshard/pkg/central/charts/data/tenant-resources/Chart.yaml delete mode 100644 fleetshard/pkg/central/charts/data/tenant-resources/README.md delete mode 100644 fleetshard/pkg/central/charts/data/tenant-resources/config/default-annotations.yaml.tpl delete mode 100644 fleetshard/pkg/central/charts/data/tenant-resources/config/default-labels.yaml.tpl delete mode 100644 fleetshard/pkg/central/charts/data/tenant-resources/config/local-network-cidr-ranges-ipv6.yaml.tpl delete mode 100644 fleetshard/pkg/central/charts/data/tenant-resources/config/local-network-cidr-ranges.yaml.tpl delete mode 100644 fleetshard/pkg/central/charts/data/tenant-resources/templates/_helpers.tpl delete mode 100644 fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-central.yaml delete mode 100644 fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-default.yaml delete mode 100644 fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-scanner-v4.yaml delete mode 100644 fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-scanner.yaml delete mode 100644 fleetshard/pkg/central/charts/data/tenant-resources/templates/vpa-central.yaml delete mode 100644 fleetshard/pkg/central/charts/data/tenant-resources/values.yaml delete mode 100644 fleetshard/pkg/central/reconciler/tenant_chart_reconciler.go delete mode 100644 fleetshard/pkg/central/reconciler/tenant_chart_reconciler_test.go diff --git a/.openshift-ci/tests/e2e.sh b/.openshift-ci/tests/e2e.sh index a500e211ca..ed397a745f 100755 --- a/.openshift-ci/tests/e2e.sh +++ b/.openshift-ci/tests/e2e.sh @@ -98,9 +98,6 @@ if [[ "$SPAWN_LOGGER" == "true" ]]; then fi FAIL=0 -if ! "${GITROOT}/.openshift-ci/tests/netpol-test.sh"; then - FAIL=1 -fi if ! "${GITROOT}/.openshift-ci/tests/e2e-test.sh"; then FAIL=1 diff --git a/.openshift-ci/tests/netpol-test.sh b/.openshift-ci/tests/netpol-test.sh deleted file mode 100755 index 159e696e3e..0000000000 --- a/.openshift-ci/tests/netpol-test.sh +++ /dev/null @@ -1,171 +0,0 @@ -#!/usr/bin/env bash -set -eo pipefail - -GITROOT="$(git rev-parse --show-toplevel)" -export GITROOT -# shellcheck source=/dev/null -source "${GITROOT}/dev/env/scripts/lib.sh" - -SERVICE_STARTUP_WAIT_TIME="2m" -CLIENT_STARTUP_WAIT_TIME="1m" - -# Tests that a connection is allowed without network policies, and that it's disallowed after applying the policies (in either namespaces) -test_central_connectivity_from_different_namespace() { - echo "Running ${FUNCNAME[0]}" "$@" - - local CENTRAL_NS="$1" - local CLIENT_NS="$2" - local CLIENT_NAME="$3" - local SERVICE_HOST="$4" - - helm install fake-client "${GITROOT}/test/network-policy/fake-client" --set name="${CLIENT_NAME}" \ - --set service.host="${SERVICE_HOST}" --namespace "${CLIENT_NS}" --create-namespace - $KUBECTL -n "${CLIENT_NS}" wait --for=condition=Available deployment/"${CLIENT_NAME}" --timeout "${CLIENT_STARTUP_WAIT_TIME}" - - helm install client-netpol "${GITROOT}/fleetshard/pkg/central/charts/data/tenant-resources" --namespace "${CLIENT_NS}" \ - --set secureTenantNetwork=true - $KUBECTL -n "${CLIENT_NS}" wait --for=condition=Available=false deployment/"${CLIENT_NAME}" - - helm uninstall client-netpol --namespace "${CLIENT_NS}" - $KUBECTL -n "${CLIENT_NS}" wait --for=condition=Available deployment/"${CLIENT_NAME}" - - helm install central-netpol "${GITROOT}/fleetshard/pkg/central/charts/data/tenant-resources" --namespace "${CENTRAL_NS}" \ - --set secureTenantNetwork=true - $KUBECTL -n "${CLIENT_NS}" wait --for=condition=Available=false deployment/"${CLIENT_NAME}" - - helm uninstall central-netpol --namespace "${CENTRAL_NS}" - helm uninstall fake-client --namespace "${CLIENT_NS}" -} - -# Tests that a connection is allowed without network policies, and that it's disallowed after applying the policies -test_central_connectivity_from_same_namespace() -{ - echo "Running ${FUNCNAME[0]}" "$@" - - local CLIENT_NS="$1" - local CLIENT_NAME="$2" - local SERVICE_HOST="$3" - - helm install fake-client "${GITROOT}/test/network-policy/fake-client" --set name="${CLIENT_NAME}" --namespace "${CLIENT_NS}" \ - --set service.host="${SERVICE_HOST}" - $KUBECTL -n "${CLIENT_NS}" wait --for=condition=Available deployment/"${CLIENT_NAME}" --timeout "${CLIENT_STARTUP_WAIT_TIME}" - - helm install tenant-netpol "${GITROOT}/fleetshard/pkg/central/charts/data/tenant-resources" --namespace "${CLIENT_NS}" \ - --set secureTenantNetwork=true - $KUBECTL -n "${CLIENT_NS}" wait --for=condition=Available=false deployment/"${CLIENT_NAME}" - - helm uninstall tenant-netpol --namespace "${CLIENT_NS}" - helm uninstall fake-client --namespace "${CLIENT_NS}" -} - -# Tests that a connection is allowed even with the network policies applied -test_central_connection_allowed() -{ - echo "Running ${FUNCNAME[0]}" "$@" - - local CLIENT_NS="$1" - local CLIENT_NAME="$2" - local SERVICE_HOST="$3" - - helm install tenant-netpol "${GITROOT}/fleetshard/pkg/central/charts/data/tenant-resources" --namespace "${CLIENT_NS}" \ - --set secureTenantNetwork=true - helm install fake-client "${GITROOT}/test/network-policy/fake-client" --set name="${CLIENT_NAME}" --namespace "${CLIENT_NS}" \ - --set service.host="${SERVICE_HOST}" - $KUBECTL -n "${CLIENT_NS}" wait --for=condition=Available deployment/"${CLIENT_NAME}" --timeout "${CLIENT_STARTUP_WAIT_TIME}" - - helm uninstall tenant-netpol --namespace "${CLIENT_NS}" - helm uninstall fake-client --namespace "${CLIENT_NS}" -} - -# Tests connectivity to a fake Central from various different sources -test_connectivity_to_central() { - local CENTRAL_NS="rhacs-fake-service" - local CLIENT_NS="rhacs-fake-client" - - helm install fake-central "${GITROOT}/test/network-policy/fake-service" --namespace "${CENTRAL_NS}" --create-namespace - $KUBECTL -n "${CENTRAL_NS}" wait --for=condition=Available deployment/central --timeout "${SERVICE_STARTUP_WAIT_TIME}" - - # use the IP to make sure access is denied because of the policy (as opposed to the client not having DNS access) - local SERVICE_IP - SERVICE_IP=$($KUBECTL -n rhacs-fake-service get service central-service -o jsonpath='{.spec.clusterIP}') - - # no connections between tenant namespaces should be allowed - test_central_connectivity_from_different_namespace "${CENTRAL_NS}" "${CLIENT_NS}" central "${SERVICE_IP}" - test_central_connectivity_from_different_namespace "${CENTRAL_NS}" "${CLIENT_NS}" scanner "${SERVICE_IP}" - test_central_connectivity_from_different_namespace "${CENTRAL_NS}" "${CLIENT_NS}" scanner-db "${SERVICE_IP}" - test_central_connectivity_from_different_namespace "${CENTRAL_NS}" "${CLIENT_NS}" scanner-v4-indexer "${SERVICE_IP}" - test_central_connectivity_from_different_namespace "${CENTRAL_NS}" "${CLIENT_NS}" scanner-v4-matcher "${SERVICE_IP}" - test_central_connectivity_from_different_namespace "${CENTRAL_NS}" "${CLIENT_NS}" scanner-v4-db "${SERVICE_IP}" - test_central_connectivity_from_different_namespace "${CENTRAL_NS}" "${CLIENT_NS}" other-app "${SERVICE_IP}" - - # connections from these apps should be allowed within the same namespace - test_central_connection_allowed "${CENTRAL_NS}" scanner "${SERVICE_IP}" - test_central_connection_allowed "${CENTRAL_NS}" scanner-v4-indexer "${SERVICE_IP}" - test_central_connection_allowed "${CENTRAL_NS}" scanner-v4-matcher "${SERVICE_IP}" - - # connections from these apps should *not* be allowed within the same namespace - test_central_connectivity_from_same_namespace "${CENTRAL_NS}" scanner-db "${SERVICE_IP}" - test_central_connectivity_from_same_namespace "${CENTRAL_NS}" scanner-v4-db "${SERVICE_IP}" - test_central_connectivity_from_same_namespace "${CENTRAL_NS}" other-app "${SERVICE_IP}" - - $KUBECTL delete ns "${CENTRAL_NS}" - $KUBECTL delete ns "${CLIENT_NS}" -} - -test_central_connectivity_to_different_namespace() { - echo "Running ${FUNCNAME[0]}" "$@" - - local SERVICE_NS="$1" - local CENTRAL_NS="$2" - local SERVICE_NAME="$3" - - helm install fake-service "${GITROOT}/test/network-policy/fake-service" --namespace "${SERVICE_NS}" --create-namespace \ - --set name="${SERVICE_NAME}" - $KUBECTL -n "${SERVICE_NS}" wait --for=condition=Available deployment/"${SERVICE_NAME}" --timeout "${SERVICE_STARTUP_WAIT_TIME}" - - # use the IP to make sure access is denied because of the policy (as opposed to the client not having DNS access) - local SERVICE_IP - SERVICE_IP=$($KUBECTL -n "${SERVICE_NS}" get service "${SERVICE_NAME}"-service -o jsonpath='{.spec.clusterIP}') - - helm install fake-central "${GITROOT}/test/network-policy/fake-client" --set name=central --namespace "${CENTRAL_NS}" \ - --create-namespace --set service.host="${SERVICE_IP}" - $KUBECTL -n "${CENTRAL_NS}" wait --for=condition=Available deployment/central --timeout "${CLIENT_STARTUP_WAIT_TIME}" - - helm install central-netpol "${GITROOT}/fleetshard/pkg/central/charts/data/tenant-resources" --namespace "${CENTRAL_NS}" \ - --set secureTenantNetwork=true - $KUBECTL -n "${CENTRAL_NS}" wait --for=condition=Available=false deployment/central - - helm uninstall central-netpol --namespace "${CENTRAL_NS}" - helm uninstall fake-central --namespace "${CENTRAL_NS}" - helm uninstall fake-service --namespace "${SERVICE_NS}" -} - -test_central_connectivity_into_same_namespace() { - test_central_connectivity_to_different_namespace "$1" "$1" "$2" -} - -# Tests connectivity from a fake Central to different destinations -test_connectivity_from_central() { - local SERVICE_NS="rhacs-fake-service" - local CLIENT_NS="rhacs-fake-client" - - # no connections between tenant namespaces should be allowed - test_central_connectivity_to_different_namespace "${SERVICE_NS}" "${CLIENT_NS}" central - test_central_connectivity_to_different_namespace "${SERVICE_NS}" "${CLIENT_NS}" scanner - test_central_connectivity_to_different_namespace "${SERVICE_NS}" "${CLIENT_NS}" scanner-db - test_central_connectivity_to_different_namespace "${SERVICE_NS}" "${CLIENT_NS}" scanner-v4-indexer - test_central_connectivity_to_different_namespace "${SERVICE_NS}" "${CLIENT_NS}" scanner-v4-matcher - test_central_connectivity_to_different_namespace "${SERVICE_NS}" "${CLIENT_NS}" scanner-v4-db - test_central_connectivity_to_different_namespace "${SERVICE_NS}" "${CLIENT_NS}" other-app - - # connections to these apps should not be allowed within the same namespace - test_central_connectivity_into_same_namespace "${SERVICE_NS}" scanner-db - test_central_connectivity_into_same_namespace "${SERVICE_NS}" scanner-v4-db - test_central_connectivity_into_same_namespace "${SERVICE_NS}" other-app - - $KUBECTL delete ns "${SERVICE_NS}" - $KUBECTL delete ns "${CLIENT_NS}" -} - -test_connectivity_to_central -test_connectivity_from_central diff --git a/fleetshard/pkg/central/charts/charts.go b/fleetshard/pkg/central/charts/charts.go index b5ac614041..2c61d88489 100644 --- a/fleetshard/pkg/central/charts/charts.go +++ b/fleetshard/pkg/central/charts/charts.go @@ -25,7 +25,7 @@ var ( // The templates/* entry is necessary because files starting with an underscore are only embedded when matched // via *, not when recursively traversing a directory. Once we switch to go1.18, we can change the embed spec // to all:data. - //go:embed data data/tenant-resources/templates/* + //go:embed data/* data embed.FS ) @@ -120,15 +120,6 @@ func GetChart(name string, urls []string) (*chart.Chart, error) { return loadedChart, nil } -// MustGetChart loads a chart from the data directory. Unlike GetChart, it panics if an error is encountered. -func MustGetChart(name string, urls []string) *chart.Chart { - chrt, err := GetChart(name, urls) - if err != nil { - panic(err) - } - return chrt -} - // InstallOrUpdateChart installs a new object from helm chart or update an existing object with the same Name, Namespace and Kind func InstallOrUpdateChart(ctx context.Context, obj *unstructured.Unstructured, client ctrlClient.Client) error { key := ctrlClient.ObjectKey{Namespace: obj.GetNamespace(), Name: obj.GetName()} diff --git a/fleetshard/pkg/central/charts/charts_test.go b/fleetshard/pkg/central/charts/charts_test.go index 0e810345da..2cb640a1d1 100644 --- a/fleetshard/pkg/central/charts/charts_test.go +++ b/fleetshard/pkg/central/charts/charts_test.go @@ -33,12 +33,6 @@ var dummyDeployment = &appsv1.Deployment{ }, } -func TestTenantResourcesChart(t *testing.T) { - c, err := GetChart("tenant-resources", nil) - require.NoError(t, err) - assert.NotNil(t, c) -} - func TestInstallOrUpdateChartCreateNew(t *testing.T) { chart := mustGetChart(t, "test-chart") fakeClient := testutils.NewFakeClientBuilder(t).Build() diff --git a/fleetshard/pkg/central/charts/data/tenant-resources/Chart.yaml b/fleetshard/pkg/central/charts/data/tenant-resources/Chart.yaml deleted file mode 100644 index 9e91c580af..0000000000 --- a/fleetshard/pkg/central/charts/data/tenant-resources/Chart.yaml +++ /dev/null @@ -1,6 +0,0 @@ -apiVersion: v2 -name: central-tenant-resources -description: Helm Chart for Central Tenant Resources in Fleetshard -type: application -version: 0.0.0 -appVersion: 0.0.0 diff --git a/fleetshard/pkg/central/charts/data/tenant-resources/README.md b/fleetshard/pkg/central/charts/data/tenant-resources/README.md deleted file mode 100644 index 8390646019..0000000000 --- a/fleetshard/pkg/central/charts/data/tenant-resources/README.md +++ /dev/null @@ -1,3 +0,0 @@ -# Network Policies - -Regarding the Tenant's Network Policies, we must respect the ACS Architecture (see https://docs.openshift.com/acs/4.4/architecture/acs-architecture.html#acs-architecture-interaction-between-services_acs-architecture). diff --git a/fleetshard/pkg/central/charts/data/tenant-resources/config/default-annotations.yaml.tpl b/fleetshard/pkg/central/charts/data/tenant-resources/config/default-annotations.yaml.tpl deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/fleetshard/pkg/central/charts/data/tenant-resources/config/default-labels.yaml.tpl b/fleetshard/pkg/central/charts/data/tenant-resources/config/default-labels.yaml.tpl deleted file mode 100644 index 5fe9b9ba77..0000000000 --- a/fleetshard/pkg/central/charts/data/tenant-resources/config/default-labels.yaml.tpl +++ /dev/null @@ -1,4 +0,0 @@ -app.kubernetes.io/name: {{ .Chart.Name }} -helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }} -app.kubernetes.io/managed-by: {{ .Release.Service }} -app.kubernetes.io/instance: {{ .Release.Name }} diff --git a/fleetshard/pkg/central/charts/data/tenant-resources/config/local-network-cidr-ranges-ipv6.yaml.tpl b/fleetshard/pkg/central/charts/data/tenant-resources/config/local-network-cidr-ranges-ipv6.yaml.tpl deleted file mode 100644 index 3497c3b949..0000000000 --- a/fleetshard/pkg/central/charts/data/tenant-resources/config/local-network-cidr-ranges-ipv6.yaml.tpl +++ /dev/null @@ -1,2 +0,0 @@ -- fc00::/7 # RFC 4193 local private network range -- fe80::/10 # RFC 4291 link-local (directly plugged) machines diff --git a/fleetshard/pkg/central/charts/data/tenant-resources/config/local-network-cidr-ranges.yaml.tpl b/fleetshard/pkg/central/charts/data/tenant-resources/config/local-network-cidr-ranges.yaml.tpl deleted file mode 100644 index f61ff73830..0000000000 --- a/fleetshard/pkg/central/charts/data/tenant-resources/config/local-network-cidr-ranges.yaml.tpl +++ /dev/null @@ -1,6 +0,0 @@ -- 0.0.0.0/8 # RFC 1122 "this" network (LAN) -- 10.0.0.0/8 # RFC 1918 local private network (LAN) -- 100.64.0.0/10 # RFC 6598 shared address space (CGN) -- 169.254.0.0/16 # RFC 3927 link-local (directly plugged) machines -- 172.16.0.0/12 # RFC 1918 local private network (LAN) -- 192.168.0.0/16 # RFC 1918 local private network (LAN) diff --git a/fleetshard/pkg/central/charts/data/tenant-resources/templates/_helpers.tpl b/fleetshard/pkg/central/charts/data/tenant-resources/templates/_helpers.tpl deleted file mode 100644 index 501cf924e8..0000000000 --- a/fleetshard/pkg/central/charts/data/tenant-resources/templates/_helpers.tpl +++ /dev/null @@ -1,19 +0,0 @@ -{{- define "labels" -}} -{{- $labels := tpl (.Files.Get "config/default-labels.yaml.tpl") . | fromYaml -}} -{{- $labels = merge (deepCopy .Values.labels) $labels -}} -{{- $labels | toYaml | nindent 0 }} -{{- end -}} - -{{- define "annotations" -}} -{{- $annotations := tpl (.Files.Get "config/default-annotations.yaml.tpl") . | fromYaml -}} -{{- $annotations = merge (deepCopy .Values.annotations) $annotations -}} -{{- $annotations | toYaml | nindent 0 }} -{{- end -}} - -{{- define "localNetworkCidrRanges" -}} -{{- tpl (.Files.Get "config/local-network-cidr-ranges.yaml.tpl") . -}} -{{- end -}} - -{{- define "localNetworkCidrRangesIPv6" -}} -{{- tpl (.Files.Get "config/local-network-cidr-ranges-ipv6.yaml.tpl") . -}} -{{- end -}} diff --git a/fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-central.yaml b/fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-central.yaml deleted file mode 100644 index 6b40486008..0000000000 --- a/fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-central.yaml +++ /dev/null @@ -1,112 +0,0 @@ -{{- if .Values.secureTenantNetwork }} -# Source: https://docs.openshift.com/container-platform/4.13/networking/network_policy/about-network-policy.html#nw-networkpolicy-allow-from-router_about-network-policy - -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: tenant-central - labels: - {{- include "labels" . | nindent 4 }} - annotations: - {{- include "annotations" . | nindent 4 }} -spec: - podSelector: - matchLabels: - app: central - policyTypes: - - Ingress - - Egress - ingress: - - from: # Allow ingress from external Internet to use Central - - namespaceSelector: - matchLabels: - network.openshift.io/policy-group: ingress - ports: - - port: 8443 - protocol: TCP - - from: # Allow ingress from appropriate scanner pieces - - podSelector: - matchExpressions: - - { key: app, operator: In, values: [scanner, scanner-v4-indexer, scanner-v4-matcher] } - ports: - - port: 8443 - protocol: TCP - - from: # Allow ingress from fleetshard-sync for "readiness" check on Central - ensuring auth provider exists - - namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: rhacs - - podSelector: - matchLabels: - app: fleetshard-sync - ports: - - port: 8443 - protocol: TCP - - from: # Allow ingress from observability to scrape metrics - - namespaceSelector: - matchExpressions: - - { key: kubernetes.io/metadata.name, operator: In, values: [ rhacs-observability, openshift-monitoring ] } - ports: - - port: 9090 - protocol: TCP - - port: 9091 - protocol: TCP - egress: - - to: # Allow egress to all RDS instances. EgressFirewall will limit to its specific instance because the IP address is not static and EgressFirewall can do DNS resolution - - ipBlock: - cidr: {{ .Values.centralRdsCidrBlock }} - ports: - - port: 5432 - protocol: TCP - - to: # Allow egress to Scanner - - podSelector: - matchExpressions: - - { key: app, operator: In, values: [scanner, scanner-v4-indexer, scanner-v4-matcher] } - ports: - - port: 8080 - protocol: TCP - - port: 8443 - protocol: TCP - - to: # Allow egress to audit-logs-aggregator - - namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: rhacs-audit-logs - podSelector: - matchLabels: - app: audit-logs-aggregator - ports: - - port: 8888 - protocol: TCP - - to: # Allow egress to Kube API for debug snapshot generation, mTLS setup for OpenShift monitoring, telemetry - - namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: openshift-kube-apiserver - podSelector: - matchLabels: - app: openshift-kube-apiserver - ports: - - port: 443 - protocol: TCP - - port: 6443 - protocol: TCP - - to: # Allow egress to emailsender - - namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: rhacs - podSelector: - matchLabels: - app: emailsender - ports: - - port: 8080 - protocol: TCP - - port: 443 - protocol: TCP - - to: # Allow egress to external Internet - - ipBlock: - cidr: 0.0.0.0/0 - except: - {{- include "localNetworkCidrRanges" . | nindent 14 }} - - ipBlock: - cidr: ::/0 - except: - {{- include "localNetworkCidrRangesIPv6" . | nindent 14 }} -{{ end }} diff --git a/fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-default.yaml b/fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-default.yaml deleted file mode 100644 index 6f3f7a3c68..0000000000 --- a/fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-default.yaml +++ /dev/null @@ -1,37 +0,0 @@ -{{- if .Values.secureTenantNetwork }} -# If no ingress NetworkPolicy exists for a pod, then all ingress is allowed. -# Similarly, if no egress NetworkPolicy exists for a pod, all egress is allowed. -# By creating a policy targeting all pods, we deny all ingress and egress that -# is not explicitly allowed by other NetworkPolicy. - -# Following https://docs.openshift.com/container-platform/4.14/networking/network_policy/about-network-policy.html#nw-networkpolicy-optimize-ovn_about-network-policy -# we allow any DNS in the cluster, including OpenShift Internal, KubeDNS, and -# external DNS traffic. -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: default-deny-all-except-dns - labels: - {{- include "labels" . | nindent 4 }} - annotations: - {{- include "annotations" . | nindent 4 }} -spec: - podSelector: {} - policyTypes: - - Ingress - - Egress - egress: - - to: - - namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: openshift-dns - ports: - - port: 53 - protocol: TCP - - port: 53 - protocol: UDP - - port: 5353 - protocol: TCP - - port: 5353 - protocol: UDP -{{ end }} diff --git a/fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-scanner-v4.yaml b/fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-scanner-v4.yaml deleted file mode 100644 index 2cb4ae0a6e..0000000000 --- a/fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-scanner-v4.yaml +++ /dev/null @@ -1,162 +0,0 @@ -{{- if .Values.secureTenantNetwork }} -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: tenant-scanner-v4-indexer - labels: - {{- include "labels" . | nindent 4 }} - annotations: - {{- include "annotations" . | nindent 4 }} -spec: - podSelector: - matchLabels: - app: scanner-v4-indexer - ingress: - - from: # Allow ingress from Central and Matcher - - podSelector: - matchExpressions: - - { key: app, operator: In, values: [central, scanner-v4-matcher] } - ports: - - port: 8443 - protocol: TCP - - from: # Allow ingress from observability to scrape metrics - - namespaceSelector: - matchExpressions: - - { key: kubernetes.io/metadata.name, operator: In, values: [ rhacs-observability, openshift-monitoring ] } - ports: - - port: 9090 - protocol: TCP - - port: 9091 - protocol: TCP - egress: - - to: # Allow egress to Scanner-v4-db for normal function - - podSelector: - matchLabels: - app: scanner-v4-db - ports: - - port: 5432 - protocol: TCP - - to: # Allow egress to Central for periodic file updates - - podSelector: - matchLabels: - app: central - ports: - - port: 8443 - protocol: TCP - - to: # Allow egress to external Internet for Image Registries - - ipBlock: - cidr: 0.0.0.0/0 - except: - {{- include "localNetworkCidrRanges" . | nindent 14 }} - - ipBlock: - cidr: ::/0 - except: - {{- include "localNetworkCidrRangesIPv6" . | nindent 14 }} - - to: # Allow egress to Kube API for mTLS setup for OpenShift monitoring - - namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: openshift-kube-apiserver - podSelector: - matchLabels: - app: openshift-kube-apiserver - ports: - - port: 443 - protocol: TCP - - port: 6443 - protocol: TCP - policyTypes: - - Ingress - - Egress ---- -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: tenant-scanner-v4-matcher - labels: - {{- include "labels" . | nindent 4 }} - annotations: - {{- include "annotations" . | nindent 4 }} -spec: - podSelector: - matchLabels: - app: scanner-v4-matcher - ingress: - - from: # Allow ingress from Central - - podSelector: - matchLabels: - app: central - ports: - - port: 8443 - protocol: TCP - - from: # Allow ingress from observability to scrape metrics - - namespaceSelector: - matchExpressions: - - { key: kubernetes.io/metadata.name, operator: In, values: [ rhacs-observability, openshift-monitoring ] } - ports: - - port: 9090 - protocol: TCP - - port: 9091 - protocol: TCP - egress: - - to: # Allow egress to Scanner-v4-db for normal function - - podSelector: - matchLabels: - app: scanner-v4-db - ports: - - port: 5432 - protocol: TCP - - to: # Allow egress to indexer for normal function, and Central for periodic file updates - - podSelector: - matchExpressions: - - { key: app, operator: In, values: [central, scanner-v4-indexer] } - ports: - - port: 8443 - protocol: TCP - - to: # Allow egress to Kube API for mTLS setup for OpenShift monitoring - - namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: openshift-kube-apiserver - podSelector: - matchLabels: - app: openshift-kube-apiserver - ports: - - port: 443 - protocol: TCP - - port: 6443 - protocol: TCP - policyTypes: - - Ingress - - Egress ---- -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: tenant-scanner-v4-db - labels: - {{- include "labels" . | nindent 4 }} - annotations: - {{- include "annotations" . | nindent 4 }} -spec: - podSelector: - matchLabels: - app: scanner-v4-db - ingress: - - from: # Allow ingress from Indexer and Matcher - - podSelector: - matchExpressions: - - { key: app, operator: In, values: [scanner-v4-indexer, scanner-v4-matcher] } - ports: - - port: 5432 - protocol: TCP - - from: # Allow ingress from observability to scrape metrics - - namespaceSelector: - matchExpressions: - - { key: kubernetes.io/metadata.name, operator: In, values: [ rhacs-observability, openshift-monitoring ] } - ports: - - port: 9090 - protocol: TCP - - port: 9091 - protocol: TCP - policyTypes: - - Ingress -{{ end }} diff --git a/fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-scanner.yaml b/fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-scanner.yaml deleted file mode 100644 index fb523d571c..0000000000 --- a/fleetshard/pkg/central/charts/data/tenant-resources/templates/network-policy-scanner.yaml +++ /dev/null @@ -1,83 +0,0 @@ -{{- if .Values.secureTenantNetwork }} -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: tenant-scanner - labels: - {{- include "labels" . | nindent 4 }} - annotations: - {{- include "annotations" . | nindent 4 }} -spec: - podSelector: - matchLabels: - app: scanner - ingress: - - from: # Allow ingress from Central to use Scanner - - podSelector: - matchLabels: - app: central - ports: - - port: 8080 - protocol: TCP - - port: 8443 - protocol: TCP - - from: # Allow ingress from observability to scrape metrics - - namespaceSelector: - matchExpressions: - - { key: kubernetes.io/metadata.name, operator: In, values: [ rhacs-observability, openshift-monitoring ] } - ports: - - port: 9090 - protocol: TCP - - port: 9091 - protocol: TCP - egress: - - to: # Allow egress to Central for vulnerability data updates - - podSelector: - matchLabels: - app: central - ports: - - port: 8443 - protocol: TCP - - to: # Allow egress to Scanner-db - - podSelector: - matchLabels: - app: scanner-db - ports: - - port: 5432 - protocol: TCP - - to: # Allow egress to external Internet for Image Registries - - ipBlock: - cidr: 0.0.0.0/0 - except: - {{- include "localNetworkCidrRanges" . | nindent 14 }} - - ipBlock: - cidr: ::/0 - except: - {{- include "localNetworkCidrRangesIPv6" . | nindent 14 }} - policyTypes: - - Ingress - - Egress ---- -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: tenant-scanner-db - labels: - {{- include "labels" . | nindent 4 }} - annotations: - {{- include "annotations" . | nindent 4 }} -spec: - podSelector: - matchLabels: - app: scanner-db - ingress: - - from: # Allow ingress from scanner to use scanner-db - - podSelector: - matchLabels: - app: scanner - ports: - - port: 5432 - protocol: TCP - policyTypes: - - Ingress -{{ end }} diff --git a/fleetshard/pkg/central/charts/data/tenant-resources/templates/vpa-central.yaml b/fleetshard/pkg/central/charts/data/tenant-resources/templates/vpa-central.yaml deleted file mode 100644 index bf52ea4546..0000000000 --- a/fleetshard/pkg/central/charts/data/tenant-resources/templates/vpa-central.yaml +++ /dev/null @@ -1,22 +0,0 @@ -{{- if ((.Values.verticalPodAutoscalers).central).enabled }} -apiVersion: autoscaling.k8s.io/v1 -kind: VerticalPodAutoscaler -metadata: - name: central-vpa - labels: {{ include "labels" . | nindent 4 }} - annotations: {{ include "annotations" . | nindent 4 }} -spec: - targetRef: - apiVersion: "apps/v1" - kind: "Deployment" - name: "central" - {{- if .Values.verticalPodAutoscalers.central.updatePolicy }} - updatePolicy: {{ toYaml .Values.verticalPodAutoscalers.central.updatePolicy | nindent 4 }} - {{- end }} - {{- if .Values.verticalPodAutoscalers.central.resourcePolicy }} - resourcePolicy: {{ toYaml .Values.verticalPodAutoscalers.central.resourcePolicy | nindent 4 }} - {{- end }} - {{- if .Values.verticalPodAutoscalers.central.recommenders }} - recommenders: {{ toYaml .Values.verticalPodAutoscalers.central.recommenders | nindent 4 }} - {{- end }} -{{ end }} diff --git a/fleetshard/pkg/central/charts/data/tenant-resources/values.yaml b/fleetshard/pkg/central/charts/data/tenant-resources/values.yaml deleted file mode 100644 index fb6c506a13..0000000000 --- a/fleetshard/pkg/central/charts/data/tenant-resources/values.yaml +++ /dev/null @@ -1,7 +0,0 @@ -labels: {} -annotations: {} -secureTenantNetwork: false -centralRdsCidrBlock: "10.1.0.0/16" -verticalPodAutoscalers: - central: - enabled: false diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index 2916f6f75d..2b14eb88bc 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/fleetshard/pkg/central/reconciler/reconciler.go @@ -18,7 +18,6 @@ import ( openshiftRouteV1 "github.com/openshift/api/route/v1" "github.com/pkg/errors" "github.com/stackrox/acs-fleet-manager/fleetshard/config" - "github.com/stackrox/acs-fleet-manager/fleetshard/pkg/central/charts" "github.com/stackrox/acs-fleet-manager/fleetshard/pkg/central/cloudprovider" "github.com/stackrox/acs-fleet-manager/fleetshard/pkg/central/postgres" "github.com/stackrox/acs-fleet-manager/fleetshard/pkg/cipher" @@ -34,7 +33,6 @@ import ( "github.com/stackrox/rox/pkg/random" "golang.org/x/exp/maps" "gopkg.in/yaml.v2" - "helm.sh/helm/v3/pkg/chart" corev1 "k8s.io/api/core/v1" apiErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -52,8 +50,6 @@ const ( PauseReconcileAnnotation = "stackrox.io/pause-reconcile" - helmReleaseName = "tenant-resources" - argoCdManagedBy = "argocd.argoproj.io/managed-by" openshiftGitopsNamespace = "openshift-gitops" @@ -141,7 +137,6 @@ type CentralReconciler struct { useRoutes bool Resources bool namespaceReconciler *namespaceReconciler - tenantChartReconciler *tenantChartReconciler centralCrReconciler *centralCrReconciler argoReconciler *argoReconciler tenantCleanup *TenantCleanup @@ -158,8 +153,6 @@ type CentralReconciler struct { managedDBProvisioningClient cloudprovider.DBClient managedDBInitFunc postgres.CentralDBInitFunc - resourcesChart *chart.Chart - wantsAuthProvider bool hasAuthProvider bool verifyAuthProviderFunc verifyAuthProviderExistsFunc @@ -269,10 +262,6 @@ func (r *CentralReconciler) Reconcile(ctx context.Context, remoteCentral private return nil, errors.New("ArgoCD application not yet deleted") } } - - if err := r.tenantChartReconciler.ensureResourcesExist(ctx, remoteCentral); err != nil { - return nil, errors.Wrapf(err, "unable to install chart resource for central %s/%s", central.GetNamespace(), central.GetName()) - } } if err = r.reconcileDeclarativeConfigurationData(ctx, remoteCentral); err != nil { @@ -1562,8 +1551,6 @@ func (r *CentralReconciler) needsReconcile(changed bool, central *v1alpha1.Centr return ok && forceReconcile == "true" } -var resourcesChart = charts.MustGetChart("tenant-resources", nil) - func (r *CentralReconciler) checkSecretExists( ctx context.Context, remoteCentralNamespace string, @@ -1700,7 +1687,6 @@ func NewCentralReconciler(k8sClient ctrlClient.Client, fleetmanagerClient *fleet opts CentralReconcilerOptions, ) *CentralReconciler { nsReconciler := newNamespaceReconciler(k8sClient) - chartReconciler := newTenantChartReconciler(k8sClient, opts.SecureTenantNetwork) crReconciler := newCentralCrReconciler(k8sClient) argoReconciler := newArgoReconciler(k8sClient, opts.ArgoReconcilerOptions) r := &CentralReconciler{ @@ -1711,7 +1697,6 @@ func NewCentralReconciler(k8sClient ctrlClient.Client, fleetmanagerClient *fleet useRoutes: opts.UseRoutes, wantsAuthProvider: opts.WantsAuthProvider, namespaceReconciler: nsReconciler, - tenantChartReconciler: chartReconciler, centralCrReconciler: crReconciler, argoReconciler: argoReconciler, tenantCleanup: NewTenantCleanup(k8sClient, TenantCleanupOptions{SecureTenantNetwork: opts.SecureTenantNetwork, ArgoReconcilerOptions: opts.ArgoReconcilerOptions}), @@ -1731,8 +1716,7 @@ func NewCentralReconciler(k8sClient ctrlClient.Client, fleetmanagerClient *fleet verifyAuthProviderFunc: hasAuthProvider, tenantImagePullSecret: []byte(opts.TenantImagePullSecret), - resourcesChart: resourcesChart, - clock: realClock{}, + clock: realClock{}, } r.needsReconcileFunc = r.needsReconcile diff --git a/fleetshard/pkg/central/reconciler/reconciler_test.go b/fleetshard/pkg/central/reconciler/reconciler_test.go index 9103e8d103..612e4a7100 100644 --- a/fleetshard/pkg/central/reconciler/reconciler_test.go +++ b/fleetshard/pkg/central/reconciler/reconciler_test.go @@ -10,13 +10,11 @@ import ( "testing" "time" - argocd "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/credentials/stscreds" openshiftRouteV1 "github.com/openshift/api/route/v1" "github.com/pkg/errors" "github.com/stackrox/acs-fleet-manager/fleetshard/config" - "github.com/stackrox/acs-fleet-manager/fleetshard/pkg/central/charts" "github.com/stackrox/acs-fleet-manager/fleetshard/pkg/central/cloudprovider" "github.com/stackrox/acs-fleet-manager/fleetshard/pkg/central/cloudprovider/awsclient" "github.com/stackrox/acs-fleet-manager/fleetshard/pkg/central/postgres" @@ -35,10 +33,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/yaml.v2" - "helm.sh/helm/v3/pkg/chart/loader" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" - networkingv1 "k8s.io/api/networking/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -360,7 +356,6 @@ func TestReconcileLastHashNotUpdatedOnError(t *testing.T) { }, centralDeploymentObject()).Build() nsReconciler := newNamespaceReconciler(fakeClient) - chartReconciler := newTenantChartReconciler(fakeClient, true) crReconciler := newCentralCrReconciler(fakeClient) argoReconciler := newArgoReconciler(fakeClient, ArgoReconcilerOptions{ArgoCdNamespace: "openshift-gitops"}) @@ -368,11 +363,9 @@ func TestReconcileLastHashNotUpdatedOnError(t *testing.T) { status: pointer.Int32(0), client: fakeClient, central: private.ManagedCentral{}, - resourcesChart: resourcesChart, encryptionKeyGenerator: cipher.AES256KeyGenerator{}, secretBackup: k8s.NewSecretBackup(fakeClient, false), namespaceReconciler: nsReconciler, - tenantChartReconciler: chartReconciler, centralCrReconciler: crReconciler, argoReconciler: argoReconciler, tenantCleanup: NewTenantCleanup(fakeClient, TenantCleanupOptions{}), @@ -830,44 +823,6 @@ func TestReportRoutesStatuses(t *testing.T) { assert.ElementsMatch(t, expected, actual) } -func TestChartResourcesAreAddedAndRemoved(t *testing.T) { - chartFiles, err := charts.TraverseChart(testdata, "testdata/tenant-resources") - require.NoError(t, err) - chart, err := loader.LoadFiles(chartFiles) - require.NoError(t, err) - - fakeClient, _, r := getClientTrackerAndReconciler( - t, - defaultCentralConfig, - nil, - defaultReconcilerOptions, - ) - r.tenantChartReconciler = newTenantChartReconciler(fakeClient, true).withChart(chart) - r.tenantCleanup = NewTenantCleanup(fakeClient, TenantCleanupOptions{}) - r.tenantCleanup.chartReconciler = newTenantChartReconciler(fakeClient, true).withChart(chart) - _, err = r.Reconcile(context.TODO(), simpleManagedCentral) - require.NoError(t, err) - - var dummyObj networkingv1.NetworkPolicy - dummyObjKey := client.ObjectKey{Namespace: simpleManagedCentral.Metadata.Namespace, Name: "dummy"} - err = fakeClient.Get(context.TODO(), dummyObjKey, &dummyObj) - assert.NoError(t, err) - - assert.Equal(t, k8s.ManagedByFleetshardValue, dummyObj.GetLabels()[k8s.ManagedByLabelKey]) - - deletedCentral := simpleManagedCentral - deletedCentral.Metadata.DeletionTimestamp = time.Now().Format(time.RFC3339) - - _, err = r.Reconcile(context.TODO(), deletedCentral) - for i := 0; i < 3 && errors.Is(err, ErrDeletionInProgress); i++ { - _, err = r.Reconcile(context.TODO(), deletedCentral) - } - require.NoError(t, err) - - err = fakeClient.Get(context.TODO(), dummyObjKey, &dummyObj) - assert.True(t, k8sErrors.IsNotFound(err)) -} - func TestCentralEncryptionKeyIsGenerated(t *testing.T) { fakeClient, _, r := getClientTrackerAndReconciler( t, @@ -897,111 +852,6 @@ func TestCentralEncryptionKeyIsGenerated(t *testing.T) { require.Equal(t, expectedKeyLen, len(encKey)) } -func TestChartResourcesAreAddedAndUpdated(t *testing.T) { - chartFiles, err := charts.TraverseChart(testdata, "testdata/tenant-resources") - require.NoError(t, err) - chart, err := loader.LoadFiles(chartFiles) - require.NoError(t, err) - - fakeClient, _, r := getClientTrackerAndReconciler( - t, - defaultCentralConfig, - nil, - defaultReconcilerOptions, - ) - r.tenantChartReconciler = newTenantChartReconciler(fakeClient, true).withChart(chart) - - _, err = r.Reconcile(context.TODO(), simpleManagedCentral) - require.NoError(t, err) - - var dummyObj networkingv1.NetworkPolicy - dummyObjKey := client.ObjectKey{Namespace: simpleManagedCentral.Metadata.Namespace, Name: "dummy"} - err = fakeClient.Get(context.TODO(), dummyObjKey, &dummyObj) - assert.NoError(t, err) - - dummyObj.SetAnnotations(map[string]string{"dummy-annotation": "test"}) - err = fakeClient.Update(context.TODO(), &dummyObj) - assert.NoError(t, err) - - err = fakeClient.Get(context.TODO(), dummyObjKey, &dummyObj) - assert.NoError(t, err) - assert.Equal(t, "test", dummyObj.GetAnnotations()["dummy-annotation"]) - - _, err = r.Reconcile(context.TODO(), simpleManagedCentral) - require.NoError(t, err) - err = fakeClient.Get(context.TODO(), dummyObjKey, &dummyObj) - assert.NoError(t, err) - - // verify that the chart resource was updated, by checking that the manually added annotation - // is no longer present - assert.Equal(t, "", dummyObj.GetAnnotations()["dummy-annotation"]) -} - -func TestTenantNetworkIsSecured(t *testing.T) { - fakeClient, _, r := getClientTrackerAndReconciler( - t, - defaultCentralConfig, - nil, - secureTenantNetworkReconcilerOptions, - ) - - _, err := r.Reconcile(context.TODO(), simpleManagedCentral) - require.NoError(t, err) - - expectedObjs := []client.Object{ - &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: simpleManagedCentral.Metadata.Namespace, - Name: "default-deny-all-except-dns", - }, - }, - &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: simpleManagedCentral.Metadata.Namespace, - Name: "tenant-central", - }, - }, - &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: simpleManagedCentral.Metadata.Namespace, - Name: "tenant-scanner", - }, - }, - &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: simpleManagedCentral.Metadata.Namespace, - Name: "tenant-scanner-db", - }, - }, - &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: simpleManagedCentral.Metadata.Namespace, - Name: "tenant-scanner-v4-db", - }, - }, - &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: simpleManagedCentral.Metadata.Namespace, - Name: "tenant-scanner-v4-indexer", - }, - }, - &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: simpleManagedCentral.Metadata.Namespace, - Name: "tenant-scanner-v4-matcher", - }, - }, - } - - for _, expectedObj := range expectedObjs { - actualObj := expectedObj.DeepCopyObject().(client.Object) - if !assert.NoError(t, fakeClient.Get(context.TODO(), client.ObjectKeyFromObject(expectedObj), actualObj)) { - continue - } - assert.Equal(t, k8s.ManagedByFleetshardValue, actualObj.GetLabels()[k8s.ManagedByLabelKey]) - } -} - func TestNoRoutesSentWhenOneNotCreated(t *testing.T) { _, tracker, r := getClientTrackerAndReconciler( t, @@ -2637,82 +2487,3 @@ func TestEncyrptionSHASumSameObject(t *testing.T) { require.Equal(t, sums[i-1], sums[i], "hash of the same object should always be equal but was not") } } - -func TestArgoCDApplication_CanBeToggleOnAndOff(t *testing.T) { - ctx := context.Background() - chartFiles, err := charts.TraverseChart(testdata, "testdata/tenant-resources") - require.NoError(t, err) - chart, err := loader.LoadFiles(chartFiles) - require.NoError(t, err) - managedCentral := simpleManagedCentral - - cli, _, r := getClientTrackerAndReconciler( - t, - managedCentral, - nil, - defaultReconcilerOptions, - ) - r.resourcesChart = chart - r.tenantChartReconciler.chart = chart - r.tenantCleanup.chartReconciler.chart = chart - - assertLegacyChartPresent := func(t *testing.T, present bool) { - var netPol networkingv1.NetworkPolicy - err := cli.Get(ctx, client.ObjectKey{Name: "dummy", Namespace: managedCentral.Metadata.Namespace}, &netPol) - if present { - require.NoError(t, err) - } else { - require.True(t, k8sErrors.IsNotFound(err)) - } - } - - assertArgoCdAppPresent := func(t *testing.T, present bool) { - var app argocd.Application - objectKey := r.argoReconciler.getArgoCdAppObjectKey(managedCentral.Metadata.Namespace) - err := cli.Get(ctx, objectKey, &app) - if present { - require.NoError(t, err) - } else { - require.True(t, k8sErrors.IsNotFound(err)) - } - } - - { - // Ensure argocd application is created - managedCentral.Spec.TenantResourcesValues = map[string]interface{}{ - "argoCd": map[string]interface{}{ - "enabled": true, - }, - } - _, err := r.Reconcile(ctx, managedCentral) - require.NoError(t, err) - - assertArgoCdAppPresent(t, true) - assertLegacyChartPresent(t, false) - } - - { - // Ensure argocd application is deleted - managedCentral.Spec.TenantResourcesValues = map[string]interface{}{ - "argoCd": map[string]interface{}{ - "enabled": false, - }, - } - _, err := r.Reconcile(ctx, managedCentral) - require.NoError(t, err) - - assertArgoCdAppPresent(t, false) - assertLegacyChartPresent(t, true) - } - - { - // Ensure argocd and charts application are deleted - managedCentral.Metadata.DeletionTimestamp = time.Now().Format(time.RFC3339) - _, err = r.Reconcile(ctx, managedCentral) - require.ErrorIs(t, err, ErrDeletionInProgress) - - assertArgoCdAppPresent(t, false) - assertLegacyChartPresent(t, false) - } - -} diff --git a/fleetshard/pkg/central/reconciler/tenant_chart_reconciler.go b/fleetshard/pkg/central/reconciler/tenant_chart_reconciler.go deleted file mode 100644 index 8589910937..0000000000 --- a/fleetshard/pkg/central/reconciler/tenant_chart_reconciler.go +++ /dev/null @@ -1,212 +0,0 @@ -package reconciler - -import ( - "context" - "fmt" - - "github.com/golang/glog" - "github.com/pkg/errors" - "github.com/stackrox/acs-fleet-manager/fleetshard/pkg/central/charts" - "github.com/stackrox/acs-fleet-manager/internal/dinosaur/pkg/api/private" - "github.com/stackrox/acs-fleet-manager/pkg/features" - "helm.sh/helm/v3/pkg/chart" - "helm.sh/helm/v3/pkg/chartutil" - apiErrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - ctrlClient "sigs.k8s.io/controller-runtime/pkg/client" -) - -// tenantChartReconciler provides methods to reconcile additional kubernetes resources -// required for a central tenant and defined by a helm chart -type tenantChartReconciler struct { - // client is the controller runtime client used for chart reconciliations - client ctrlClient.Client - chart *chart.Chart - secureTenantNetwork bool -} - -// newTenantChartReconciler creates a TenantChartReconciler with given arguments -// This function uses the resourceChart default -func newTenantChartReconciler(client ctrlClient.Client, secureTenantNetwork bool) *tenantChartReconciler { - return &tenantChartReconciler{client: client, chart: resourcesChart, secureTenantNetwork: secureTenantNetwork} -} - -// withChart overrides the default chart used by the TenantChartReconciler -func (r *tenantChartReconciler) withChart(c *chart.Chart) *tenantChartReconciler { - r.chart = c - return r -} - -// ensureResourcesExist installs or updates the chart for remoteCentral on the Kuberntes cluster -func (r *tenantChartReconciler) ensureResourcesExist(ctx context.Context, remoteCentral private.ManagedCentral) error { - getObjectKey := func(obj *unstructured.Unstructured) string { - return fmt.Sprintf("%s/%s/%s", - obj.GetAPIVersion(), - obj.GetKind(), - obj.GetName(), - ) - } - - vals, err := r.chartValues(remoteCentral) - if err != nil { - return fmt.Errorf("obtaining values for resources chart: %w", err) - } - - if features.PrintTenantResourcesChartValues.Enabled() { - glog.Infof("Tenant resources for central %q: %s", remoteCentral.Metadata.Name, vals) - } - - objs, err := charts.RenderToObjects(helmReleaseName, remoteCentral.Metadata.Namespace, r.chart, vals) - if err != nil { - return fmt.Errorf("rendering resources chart: %w", err) - } - - helmChartLabelValue := r.getTenantResourcesChartHelmLabelValue() - - // objectsThatShouldExist stores the keys of the objects we want to exist - var objectsThatShouldExist = map[string]struct{}{} - - for _, obj := range objs { - objectsThatShouldExist[getObjectKey(obj)] = struct{}{} - - if obj.GetNamespace() == "" { - obj.SetNamespace(remoteCentral.Metadata.Namespace) - } - if obj.GetLabels() == nil { - obj.SetLabels(map[string]string{}) - } - labels := obj.GetLabels() - labels[managedByLabelKey] = labelManagedByFleetshardValue - labels[helmChartLabelKey] = helmChartLabelValue - labels[helmChartNameLabel] = r.chart.Name() - obj.SetLabels(labels) - - objectKey := ctrlClient.ObjectKey{Namespace: remoteCentral.Metadata.Namespace, Name: obj.GetName()} - glog.Infof("Upserting object %v of type %v", objectKey, obj.GroupVersionKind()) - if err := charts.InstallOrUpdateChart(ctx, obj, r.client); err != nil { - return fmt.Errorf("Failed to upsert object %v of type %v: %w", objectKey, obj.GroupVersionKind(), err) - } - } - - // Perform garbage collection - for _, gvk := range tenantChartResourceGVKs { - gvk := gvk - var existingObjects unstructured.UnstructuredList - existingObjects.SetGroupVersionKind(gvk) - - if err := r.client.List(ctx, &existingObjects, - ctrlClient.InNamespace(remoteCentral.Metadata.Namespace), - ctrlClient.MatchingLabels{helmChartNameLabel: r.chart.Name()}, - ); err != nil { - return fmt.Errorf("failed to list tenant resources chart objects %v: %w", gvk, err) - } - - for _, existingObject := range existingObjects.Items { - // prevents erros for using address of the shared loop variable (gosec 601) by making a copy, no longer necessary once updated to Go 1.22 - existingObject := existingObject - if _, shouldExist := objectsThatShouldExist[getObjectKey(&existingObject)]; shouldExist { - continue - } - - // Re-check that the helm label is present & namespace matches. - // Failsafe against some potential k8s-client bug when listing objects with a label selector - if !r.isTenantResourcesChartObject(&existingObject, remoteCentral.Metadata.Namespace) { - glog.Infof("Object %v of type %v is not managed by the resources chart", existingObject.GetName(), gvk) - continue - } - - if existingObject.GetDeletionTimestamp() != nil { - glog.Infof("Object %v of type %v is already being deleted", existingObject.GetName(), gvk) - continue - } - - // The object exists but it should not. Delete it. - glog.Infof("Deleting object %v of type %v", existingObject.GetName(), gvk) - if err := r.client.Delete(ctx, &existingObject); err != nil { - if !apiErrors.IsNotFound(err) { - return fmt.Errorf("failed to delete central tenant object %v %q in namespace %s: %w", gvk, existingObject.GetName(), remoteCentral.Metadata.Namespace, err) - } - } - } - } - - return nil -} - -// ensureResourcesDeleted deletes all resources associated with the chart and namepsace from the Kubernetes cluster -func (r *tenantChartReconciler) ensureResourcesDeleted(ctx context.Context, namespace string) (bool, error) { - allObjectsDeleted := true - - for _, gvk := range tenantChartResourceGVKs { - gvk := gvk - var existingObjects unstructured.UnstructuredList - existingObjects.SetGroupVersionKind(gvk) - - if err := r.client.List(ctx, &existingObjects, - ctrlClient.InNamespace(namespace), - ctrlClient.MatchingLabels{helmChartNameLabel: r.chart.Name()}, - ); err != nil { - return false, fmt.Errorf("failed to list tenant resources chart objects %v in namespace %s: %w", gvk, namespace, err) - } - - for _, existingObject := range existingObjects.Items { - // prevents erros for using address of the shared loop variable (gosec 601) by making a copy, no longer necessary once updated to Go 1.22 - existingObject := existingObject - - // Re-check that the helm label is present & namespace matches. - // Failsafe against some potential k8s-client bug when listing objects with a label selector - if !r.isTenantResourcesChartObject(&existingObject, namespace) { - continue - } - - if existingObject.GetDeletionTimestamp() != nil { - allObjectsDeleted = false - continue - } - - if err := r.client.Delete(ctx, &existingObject); err != nil { - if !apiErrors.IsNotFound(err) { - return false, fmt.Errorf("failed to delete central tenant object %v in namespace %q: %w", gvk, namespace, err) - } - } - } - } - - return allObjectsDeleted, nil -} - -func (r *tenantChartReconciler) chartValues(c private.ManagedCentral) (chartutil.Values, error) { - if r.chart == nil { - return nil, errors.New("resources chart is not set") - } - src := r.chart.Values - - // We are introducing the passing of helm values from fleetManager (and gitops). If the managed central - // includes the tenant resource values, we will use them. Otherwise, defaults to the previous - // implementation. - if len(c.Spec.TenantResourcesValues) > 0 { - values := chartutil.CoalesceTables(c.Spec.TenantResourcesValues, src) - glog.Infof("Values: %v", values) - return values, nil - } - - dst := map[string]interface{}{ - "labels": stringMapToMapInterface(getTenantLabels(c)), - "annotations": stringMapToMapInterface(getTenantAnnotations(c)), - } - dst["secureTenantNetwork"] = r.secureTenantNetwork - return chartutil.CoalesceTables(dst, src), nil -} - -func (r *tenantChartReconciler) isTenantResourcesChartObject(existingObject *unstructured.Unstructured, namespace string) bool { - return existingObject.GetLabels() != nil && - existingObject.GetLabels()[helmChartNameLabel] == r.chart.Name() && - existingObject.GetLabels()[managedByLabelKey] == labelManagedByFleetshardValue && - existingObject.GetNamespace() == namespace -} - -func (r *tenantChartReconciler) getTenantResourcesChartHelmLabelValue() string { - // the objects rendered by the helm chart will have a label in the format - // helm.sh/chart: - - return fmt.Sprintf("%s-%s", r.chart.Name(), r.chart.Metadata.Version) -} diff --git a/fleetshard/pkg/central/reconciler/tenant_chart_reconciler_test.go b/fleetshard/pkg/central/reconciler/tenant_chart_reconciler_test.go deleted file mode 100644 index 3a75f4aa8c..0000000000 --- a/fleetshard/pkg/central/reconciler/tenant_chart_reconciler_test.go +++ /dev/null @@ -1,62 +0,0 @@ -package reconciler - -import ( - "testing" - - "github.com/stackrox/acs-fleet-manager/internal/dinosaur/pkg/api/private" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestChartValues(t *testing.T) { - - r := tenantChartReconciler{chart: resourcesChart} - - tests := []struct { - name string - managedCentral private.ManagedCentral - assertFn func(t *testing.T, values map[string]interface{}, err error) - }{ - { - name: "withTenantResources", - managedCentral: private.ManagedCentral{ - Spec: private.ManagedCentralAllOfSpec{ - TenantResourcesValues: map[string]interface{}{ - "verticalPodAutoscalers": map[string]interface{}{ - "central": map[string]interface{}{ - "enabled": true, - "updatePolicy": map[string]interface{}{ - "minReplicas": 1, - }, - }, - }, - }, - }, - }, - assertFn: func(t *testing.T, values map[string]interface{}, err error) { - require.NoError(t, err) - verticalPodAutoscalers, ok := values["verticalPodAutoscalers"].(map[string]interface{}) - require.True(t, ok) - central, ok := verticalPodAutoscalers["central"].(map[string]interface{}) - require.True(t, ok) - enabled, ok := central["enabled"].(bool) - require.True(t, ok) - assert.True(t, enabled) - - updatePolicy, ok := central["updatePolicy"].(map[string]interface{}) - require.True(t, ok) - minReplicas, ok := updatePolicy["minReplicas"].(int) - require.True(t, ok) - assert.Equal(t, 1, minReplicas) - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - values, err := r.chartValues(tt.managedCentral) - tt.assertFn(t, values, err) - }) - } - -} diff --git a/fleetshard/pkg/central/reconciler/tenant_cleanup.go b/fleetshard/pkg/central/reconciler/tenant_cleanup.go index cdc1330027..8278cdcb5d 100644 --- a/fleetshard/pkg/central/reconciler/tenant_cleanup.go +++ b/fleetshard/pkg/central/reconciler/tenant_cleanup.go @@ -18,11 +18,10 @@ const crNameLabelKey = "app.kubernetes.io/instance" // TenantCleanup defines methods to cleanup Kubernetes resources and namespaces for tenants // that are no longer in the list of tenants fleetshard-sync schould run on a cluster type TenantCleanup struct { - k8sClient ctrlClient.Client - chartReconciler *tenantChartReconciler - nsReconciler *namespaceReconciler - crReconciler *centralCrReconciler - argoReconciler *argoReconciler + k8sClient ctrlClient.Client + nsReconciler *namespaceReconciler + crReconciler *centralCrReconciler + argoReconciler *argoReconciler } // TenantCleanupOptions defines configuration options for the TenantCleanup logic @@ -35,11 +34,10 @@ type TenantCleanupOptions struct { // NewTenantCleanup returns a new TenantCleanup using given arguments func NewTenantCleanup(k8sClient ctrlClient.Client, opts TenantCleanupOptions) *TenantCleanup { return &TenantCleanup{ - k8sClient: k8sClient, - nsReconciler: newNamespaceReconciler(k8sClient), - crReconciler: newCentralCrReconciler(k8sClient), - chartReconciler: newTenantChartReconciler(k8sClient, opts.SecureTenantNetwork), - argoReconciler: newArgoReconciler(k8sClient, opts.ArgoReconcilerOptions), + k8sClient: k8sClient, + nsReconciler: newNamespaceReconciler(k8sClient), + crReconciler: newCentralCrReconciler(k8sClient), + argoReconciler: newArgoReconciler(k8sClient, opts.ArgoReconcilerOptions), } } @@ -89,12 +87,6 @@ func (t *TenantCleanup) DeleteK8sResources(ctx context.Context, namespace string // If any resources wouldn't be deleted by namespace deletion add them here. globalDeleted := true - deleted, err := t.chartReconciler.ensureResourcesDeleted(ctx, namespace) - if err != nil { - return false, fmt.Errorf("Failed to delete chart resources in namespace %q: %w", namespace, err) - } - globalDeleted = globalDeleted && deleted - // TODO(ROX-26277): This has to go into the tenantCleanup implementation // it is here for now for merge conflict resolution, and need additional impl before mergin ROX-26277 PR. argoCdAppDeleted, err := t.argoReconciler.ensureApplicationDeleted(ctx, namespace) @@ -103,7 +95,7 @@ func (t *TenantCleanup) DeleteK8sResources(ctx context.Context, namespace string } globalDeleted = globalDeleted && argoCdAppDeleted - deleted, err = t.crReconciler.ensureDeleted(ctx, namespace, tenantName) + deleted, err := t.crReconciler.ensureDeleted(ctx, namespace, tenantName) if err != nil { return false, fmt.Errorf("Failed to delete central CR in namespace %q: %w", namespace, err) } From 7d9f01fa6df593d09b109f9d11dc275798ea402a Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Wed, 18 Dec 2024 09:53:45 +0100 Subject: [PATCH 02/12] ROX-27129: remove tenant-resources helm chart --- fleetshard/pkg/central/reconciler/gen/main.go | 236 ------------------ .../reconciler/zzz_managed_resources.go | 14 -- 2 files changed, 250 deletions(-) delete mode 100644 fleetshard/pkg/central/reconciler/gen/main.go delete mode 100644 fleetshard/pkg/central/reconciler/zzz_managed_resources.go diff --git a/fleetshard/pkg/central/reconciler/gen/main.go b/fleetshard/pkg/central/reconciler/gen/main.go deleted file mode 100644 index 275c63c443..0000000000 --- a/fleetshard/pkg/central/reconciler/gen/main.go +++ /dev/null @@ -1,236 +0,0 @@ -// This program generates a list of GVKs used by the tenant-resources helm chart for enabling garbage collection. -// -// To enable garbage collection without this list, we would need to... -// - Manually maintain a list of GVKs present in the chart, which would be error-prone. -// - Or have the reconciler list all objects for all possible GVKs, which would be very expensive. -// - Switch to the native helm client or the helm operator, which would require a lot of changes. -// -// This program automatically generates that list. -// -// Process: -// - It extract the GVKs that are present in the tenant-resources chart manifests -// - It extract the GVKs that are already declared in the generated file -// - It combines the two lists -// - It writes the combined list to the generated file - -package main - -import ( - "fmt" - "github.com/pkg/errors" - "io/fs" - "k8s.io/apimachinery/pkg/runtime/schema" - "os" - "path" - "path/filepath" - "regexp" - "runtime" - "sort" - "strings" -) - -var ( - reconcilerDir = path.Join(path.Dir(getCurrentFile()), "..") - outFile = fmt.Sprintf("%s/zzz_managed_resources.go", reconcilerDir) - tenantResourcesChartDir = path.Join(path.Dir(getCurrentFile()), "../../charts/data/tenant-resources/templates") - gvkRegex = regexp.MustCompile(`schema.GroupVersionKind{Group: "(.*)", Version: "(.*)", Kind: "(.*)"},`) -) - -const ( - apiVersionPrefix = "apiVersion: " - kindPrefix = "kind: " -) - -func main() { - if err := generate(); err != nil { - panic(err) - } -} - -type resourceMap map[schema.GroupVersionKind]bool - -func generate() error { - - // Holds a map of GVKs that will be in the output file - // The value `bool` indicates that the resource is present in the tenant-resources helm chart - // If false, this means that the resource is only present in the generated file (for GVKs removed from the chart) - // But the GVK is still needed for garbage collection. - seen := resourceMap{} - - // Finding GVKs used in the tenant-resources chart - if err := findGVKsInChart(tenantResourcesChartDir, seen); err != nil { - return err - } - - // Finding GVKs already declared in the generated file - if err := findGVKsInGeneratedFile(seen); err != nil { - return err - } - - // Re-generating the file - if err := generateGVKsList(seen); err != nil { - return err - } - - return nil -} - -func getCurrentFile() string { - _, file, _, _ := runtime.Caller(0) - return file -} - -func generateGVKsList(seen resourceMap) error { - // Making sure resources are ordered (for deterministic output) - sorted := sortResourceKeys(seen) - - builder := strings.Builder{} - builder.WriteString("// Code generated by fleetshard/pkg/central/reconciler/gen/main.go. DO NOT EDIT.\n") - builder.WriteString("package reconciler\n\n") - builder.WriteString("import (\n") - builder.WriteString("\t\"k8s.io/apimachinery/pkg/runtime/schema\"\n") - builder.WriteString(")\n\n") - - builder.WriteString("// tenantChartResourceGVKs is a list of GroupVersionKind that...\n") - builder.WriteString("// - are present in the tenant-resources helm chart\n") - builder.WriteString("// - were present in a previous version of the chart. A comment will indicate that manual removal from the list is required.\n") - - builder.WriteString("var tenantChartResourceGVKs = []schema.GroupVersionKind{\n") - for _, k := range sorted { - builder.WriteString(fmt.Sprintf("\tschema.GroupVersionKind{Group: %q, Version: %q, Kind: %q},", k.Group, k.Version, k.Kind)) - stillInChart := seen[k] - if !stillInChart { - builder.WriteString(" // This resource was present in a previous version of the chart. Manual removal is required.") - } - builder.WriteString("\n") - } - builder.WriteString("}\n") - - genFile, err := os.Create(outFile) - if err != nil { - return err - } - defer genFile.Close() - - genFile.WriteString(builder.String()) - return nil -} - -func sortResourceKeys(seen resourceMap) []schema.GroupVersionKind { - sorted := make([]schema.GroupVersionKind, 0, len(seen)) - for k := range seen { - sorted = append(sorted, k) - } - sort.Slice(sorted, func(i, j int) bool { - left := sorted[i] - right := sorted[j] - return left.String() < right.String() - }) - return sorted -} - -func findGVKsInGeneratedFile(seen resourceMap) error { - if _, err := os.Stat(outFile); err != nil { - if os.IsNotExist(err) { - return nil - } - return err - } - - file, err := os.Open(outFile) - if err != nil { - return err - } - defer file.Close() - - fileBytes, err := os.ReadFile(outFile) - if err != nil { - return err - } - - lines := strings.Split(string(fileBytes), "\n") - - for _, line := range lines { - matches := gvkRegex.FindStringSubmatch(line) - if len(matches) != 4 { - continue - } - gvk := schema.GroupVersionKind{Group: matches[1], Version: matches[2], Kind: matches[3]} - isAlreadyPresent := seen[gvk] - seen[gvk] = isAlreadyPresent - } - return nil -} - -func findGVKsInChart(chartDir string, seen resourceMap) error { - if err := filepath.WalkDir(chartDir, func(path string, d fs.DirEntry, err error) error { - if d.IsDir() { - return nil - } - - ext := filepath.Ext(path) - if ext != ".yaml" && ext != ".yml" { - return nil - } - - fileBytes, err := os.ReadFile(path) - if err != nil { - return err - } - - if err := findGVKsInFile(fileBytes, seen); err != nil { - return fmt.Errorf("failed to parse file %q: %w", path, err) - } - - return nil - }); err != nil { - return err - } - return nil -} - -func splitFileIntoResources(fileBytes []byte) []string { - fileStr := string(fileBytes) - return strings.Split(fileStr, "---") -} - -func findGVKsInFile(fileBytes []byte, seen resourceMap) error { - resources := splitFileIntoResources(fileBytes) - for _, resource := range resources { - gvk, ok, err := findResourceGVK(resource) - if err != nil { - return fmt.Errorf("failed to parse resource: %w", err) - } - if !ok { - return errors.New("resource GVK could not be parsed") - } - seen[gvk] = true - } - return nil -} - -func findResourceGVK(resource string) (schema.GroupVersionKind, bool, error) { - lines := strings.Split(resource, "\n") - apiVersion := "" - kind := "" - for i := 0; i < len(lines); i++ { - if strings.HasPrefix(lines[i], apiVersionPrefix) { - apiVersion = strings.TrimSpace(strings.TrimPrefix(lines[i], apiVersionPrefix)) - continue - } - if strings.HasPrefix(lines[i], kindPrefix) { - kind = strings.TrimSpace(strings.TrimPrefix(lines[i], kindPrefix)) - continue - } - } - if len(apiVersion) == 0 || len(kind) == 0 { - return schema.GroupVersionKind{}, false, nil - } - - groupVersion, err := schema.ParseGroupVersion(apiVersion) - if err != nil { - return schema.GroupVersionKind{}, false, err - } - - return groupVersion.WithKind(kind), true, nil -} diff --git a/fleetshard/pkg/central/reconciler/zzz_managed_resources.go b/fleetshard/pkg/central/reconciler/zzz_managed_resources.go deleted file mode 100644 index b420229a04..0000000000 --- a/fleetshard/pkg/central/reconciler/zzz_managed_resources.go +++ /dev/null @@ -1,14 +0,0 @@ -// Code generated by fleetshard/pkg/central/reconciler/gen/main.go. DO NOT EDIT. -package reconciler - -import ( - "k8s.io/apimachinery/pkg/runtime/schema" -) - -// tenantChartResourceGVKs is a list of GroupVersionKind that... -// - are present in the tenant-resources helm chart -// - were present in a previous version of the chart. A comment will indicate that manual removal from the list is required. -var tenantChartResourceGVKs = []schema.GroupVersionKind{ - schema.GroupVersionKind{Group: "autoscaling.k8s.io", Version: "v1", Kind: "VerticalPodAutoscaler"}, - schema.GroupVersionKind{Group: "networking.k8s.io", Version: "v1", Kind: "NetworkPolicy"}, -} From f369c31bdd5543f7f711b15ae2ad7d1bc465f155 Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Wed, 18 Dec 2024 09:54:35 +0100 Subject: [PATCH 03/12] ROX-27129: remove tenant-resources helm chart --- fleetshard/pkg/central/reconciler/doc.go | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 fleetshard/pkg/central/reconciler/doc.go diff --git a/fleetshard/pkg/central/reconciler/doc.go b/fleetshard/pkg/central/reconciler/doc.go deleted file mode 100644 index cc55a2c0ee..0000000000 --- a/fleetshard/pkg/central/reconciler/doc.go +++ /dev/null @@ -1,3 +0,0 @@ -package reconciler - -//go:generate go run gen/main.go From 6f08a240a432ae965a9727499df226c91f56e6cc Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Wed, 18 Dec 2024 09:55:09 +0100 Subject: [PATCH 04/12] ROX-27129: remove tenant-resources helm chart --- fleetshard/pkg/central/reconciler/reconciler.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index 2b14eb88bc..5b70968a71 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/fleetshard/pkg/central/reconciler/reconciler.go @@ -95,9 +95,6 @@ const ( additionalAuthProviderConfigKey = "additional-auth-provider" tenantImagePullSecretName = "stackrox" // pragma: allowlist secret - - helmChartLabelKey = "helm.sh/chart" - helmChartNameLabel = "helm.sh/chart-name" ) type verifyAuthProviderExistsFunc func(ctx context.Context, central private.ManagedCentral, client ctrlClient.Client) (bool, error) From d24e441af71773c4d8493716315ec402aed44fb5 Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Wed, 18 Dec 2024 09:55:56 +0100 Subject: [PATCH 05/12] ROX-27129: remove tenant-resources helm chart --- fleetshard/pkg/central/reconciler/reconciler.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index 5b70968a71..57d683f36d 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/fleetshard/pkg/central/reconciler/reconciler.go @@ -1518,14 +1518,6 @@ func getNamespaceAnnotations(c private.ManagedCentral) map[string]string { return namespaceAnnotations } -func stringMapToMapInterface(m map[string]string) map[string]interface{} { - result := make(map[string]interface{}, len(m)) - for k, v := range m { - result[k] = v - } - return result -} - func (r *CentralReconciler) shouldSkipReadyCentral(remoteCentral private.ManagedCentral) bool { return r.wantsAuthProvider == r.hasAuthProvider && isRemoteCentralReady(&remoteCentral) From 06e21c12bd144546c1618de7b23885a77426b135 Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Wed, 18 Dec 2024 09:56:10 +0100 Subject: [PATCH 06/12] ROX-27129: remove tenant-resources helm chart --- fleetshard/pkg/central/reconciler/reconciler.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index 57d683f36d..6d39db54b5 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/fleetshard/pkg/central/reconciler/reconciler.go @@ -50,10 +50,8 @@ const ( PauseReconcileAnnotation = "stackrox.io/pause-reconcile" - argoCdManagedBy = "argocd.argoproj.io/managed-by" - openshiftGitopsNamespace = "openshift-gitops" - - centralPVCAnnotationKey = "platform.stackrox.io/obsolete-central-pvc" + argoCdManagedBy = "argocd.argoproj.io/managed-by" + managedServicesAnnotation = "platform.stackrox.io/managed-services" envAnnotationKey = "rhacs.redhat.com/environment" clusterNameAnnotationKey = "rhacs.redhat.com/cluster-name" From c66708e31a6d0403230e03e53b25533ff54e009c Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Wed, 18 Dec 2024 10:02:46 +0100 Subject: [PATCH 07/12] ROX-27129: remove tenant-resources helm chart --- fleetshard/pkg/central/reconciler/reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index 6d39db54b5..e114657546 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/fleetshard/pkg/central/reconciler/reconciler.go @@ -51,7 +51,7 @@ const ( PauseReconcileAnnotation = "stackrox.io/pause-reconcile" argoCdManagedBy = "argocd.argoproj.io/managed-by" - + managedServicesAnnotation = "platform.stackrox.io/managed-services" envAnnotationKey = "rhacs.redhat.com/environment" clusterNameAnnotationKey = "rhacs.redhat.com/cluster-name" From 2f4a6f1d9b96243d648f1d28d0acaa2b29d0b3b5 Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Wed, 18 Dec 2024 10:06:10 +0100 Subject: [PATCH 08/12] ROX-27129: remove tenant-resources helm chart --- .../pkg/central/reconciler/reconciler_test.go | 4 ---- .../reconciler/testdata/tenant-resources/Chart.yaml | 6 ------ .../testdata/tenant-resources/templates/dummy.yaml | 13 ------------- .../testdata/tenant-resources/values.yaml | 2 -- 4 files changed, 25 deletions(-) delete mode 100644 fleetshard/pkg/central/reconciler/testdata/tenant-resources/Chart.yaml delete mode 100644 fleetshard/pkg/central/reconciler/testdata/tenant-resources/templates/dummy.yaml delete mode 100644 fleetshard/pkg/central/reconciler/testdata/tenant-resources/values.yaml diff --git a/fleetshard/pkg/central/reconciler/reconciler_test.go b/fleetshard/pkg/central/reconciler/reconciler_test.go index 612e4a7100..c283b2f4ce 100644 --- a/fleetshard/pkg/central/reconciler/reconciler_test.go +++ b/fleetshard/pkg/central/reconciler/reconciler_test.go @@ -3,7 +3,6 @@ package reconciler import ( "bytes" "context" - "embed" "encoding/base64" "fmt" "net/http" @@ -130,9 +129,6 @@ metadata: }, } -//go:embed testdata -var testdata embed.FS - func createBase64Cipher(t *testing.T) cipher.Cipher { b64Cipher, err := cipher.NewLocalBase64Cipher() require.NoError(t, err, "creating base64 cipher for test") diff --git a/fleetshard/pkg/central/reconciler/testdata/tenant-resources/Chart.yaml b/fleetshard/pkg/central/reconciler/testdata/tenant-resources/Chart.yaml deleted file mode 100644 index 64e54c4d6d..0000000000 --- a/fleetshard/pkg/central/reconciler/testdata/tenant-resources/Chart.yaml +++ /dev/null @@ -1,6 +0,0 @@ -apiVersion: v2 -name: tenant-resources-test -description: Helm Chart for Testing Tenant Resources in Fleetshard -type: application -version: 0.0.0 -appVersion: 0.0.0 diff --git a/fleetshard/pkg/central/reconciler/testdata/tenant-resources/templates/dummy.yaml b/fleetshard/pkg/central/reconciler/testdata/tenant-resources/templates/dummy.yaml deleted file mode 100644 index 79ce6037c1..0000000000 --- a/fleetshard/pkg/central/reconciler/testdata/tenant-resources/templates/dummy.yaml +++ /dev/null @@ -1,13 +0,0 @@ -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: dummy - labels: - {{- .Values.labels | toYaml | nindent 4 }} - annotations: - {{- .Values.annotations | toYaml | nindent 4 }} -spec: - podSelector: {} - policyTypes: - - Ingress - - Egress diff --git a/fleetshard/pkg/central/reconciler/testdata/tenant-resources/values.yaml b/fleetshard/pkg/central/reconciler/testdata/tenant-resources/values.yaml deleted file mode 100644 index f768d01921..0000000000 --- a/fleetshard/pkg/central/reconciler/testdata/tenant-resources/values.yaml +++ /dev/null @@ -1,2 +0,0 @@ -labels: {} -annotations: {} From 02203dfce6ee5e1211dbe6cecd7536499e4d5d99 Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Wed, 18 Dec 2024 10:06:39 +0100 Subject: [PATCH 09/12] ROX-27129: remove tenant-resources helm chart --- fleetshard/pkg/central/reconciler/errors.go | 1 - fleetshard/pkg/central/reconciler/reconciler_test.go | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/fleetshard/pkg/central/reconciler/errors.go b/fleetshard/pkg/central/reconciler/errors.go index 9839c3d6f0..56ef4a1e47 100644 --- a/fleetshard/pkg/central/reconciler/errors.go +++ b/fleetshard/pkg/central/reconciler/errors.go @@ -3,7 +3,6 @@ package reconciler import "github.com/pkg/errors" var ( - errInvalidArguments = errors.New("invalid arguments") // ErrBusy returned when reconciliation for the same central is already in progress ErrBusy = errors.New("reconciler still busy") // ErrCentralNotChanged is an error returned when reconciliation runs more than once in a row with equal central diff --git a/fleetshard/pkg/central/reconciler/reconciler_test.go b/fleetshard/pkg/central/reconciler/reconciler_test.go index c283b2f4ce..c84f69a8c1 100644 --- a/fleetshard/pkg/central/reconciler/reconciler_test.go +++ b/fleetshard/pkg/central/reconciler/reconciler_test.go @@ -65,8 +65,7 @@ var ( }, } - useRoutesReconcilerOptions = CentralReconcilerOptions{UseRoutes: true} - secureTenantNetworkReconcilerOptions = CentralReconcilerOptions{SecureTenantNetwork: true} + useRoutesReconcilerOptions = CentralReconcilerOptions{UseRoutes: true} defaultAuditLogConfig = config.AuditLogging{ Enabled: true, From 98be37cf920612a276a40494be30b44d75a101ac Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Wed, 18 Dec 2024 17:50:58 +0100 Subject: [PATCH 10/12] ROX-27129: remove tenant-resources helm chart --- e2e/e2e_canary_upgrade_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/e2e/e2e_canary_upgrade_test.go b/e2e/e2e_canary_upgrade_test.go index 720d09d453..4ccfb10cac 100644 --- a/e2e/e2e_canary_upgrade_test.go +++ b/e2e/e2e_canary_upgrade_test.go @@ -587,7 +587,8 @@ labels: annotations: rhacs.redhat.com/org-name: "{{ .OrganizationName }}" secureTenantNetwork: false -centralRdsCidrBlock: "10.1.0.0/16"` +centralRdsCidrBlock: "10.1.0.0/16" +centralVpaEnabled: false` } func tenantResourcesWithCentralVpaEnabled() string { @@ -602,9 +603,7 @@ annotations: rhacs.redhat.com/org-name: "{{ .OrganizationName }}" secureTenantNetwork: false centralRdsCidrBlock: "10.1.0.0/16" -verticalPodAutoscalers: - central: - enabled: true +centralVpaEnabled: true ` } From f7f37954549da8670f1041e35b62e1920a48b94f Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Thu, 19 Dec 2024 09:06:55 +0100 Subject: [PATCH 11/12] ROX-27129: remove tenant-resources helm chart --- e2e/e2e_canary_upgrade_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/e2e/e2e_canary_upgrade_test.go b/e2e/e2e_canary_upgrade_test.go index 4ccfb10cac..bc413d2c77 100644 --- a/e2e/e2e_canary_upgrade_test.go +++ b/e2e/e2e_canary_upgrade_test.go @@ -586,6 +586,8 @@ labels: rhacs.redhat.com/instance-type: "{{ .InstanceType }}" annotations: rhacs.redhat.com/org-name: "{{ .OrganizationName }}" +argoCd: + enabled: true secureTenantNetwork: false centralRdsCidrBlock: "10.1.0.0/16" centralVpaEnabled: false` @@ -599,6 +601,8 @@ labels: rhacs.redhat.com/org-id: "{{ .OrganizationID }}" rhacs.redhat.com/tenant: "{{ .ID }}" rhacs.redhat.com/instance-type: "{{ .InstanceType }}" +argoCd: + enabled: true annotations: rhacs.redhat.com/org-name: "{{ .OrganizationName }}" secureTenantNetwork: false From e203f735bf4bf1ad098045abefdeb3b7577d14a1 Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Thu, 19 Dec 2024 10:16:07 +0100 Subject: [PATCH 12/12] ROX-27129: remove tenant-resources helm chart --- .../pkg/central/reconciler/reconciler.go | 46 +------------------ 1 file changed, 2 insertions(+), 44 deletions(-) diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index e114657546..467ad5d060 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/fleetshard/pkg/central/reconciler/reconciler.go @@ -240,23 +240,8 @@ func (r *CentralReconciler) Reconcile(ctx context.Context, remoteCentral private centralDBConnectionString = *central.Spec.Central.GetDB().ConnectionStringOverride } - if isArgoCdEnabledForTenant(remoteCentral) { - if err := r.argoReconciler.ensureApplicationExists(ctx, remoteCentral, centralDBConnectionString); err != nil { - return nil, errors.Wrapf(err, "unable to install ArgoCD application for central %s/%s", remoteCentral.Metadata.Namespace, remoteCentral.Metadata.Name) - } - } else { - - { - // This part handles the case where we enable, then disable ArgoCD for a tenant - // to make sure the ArgoCD application is cleaned up. - ok, err := r.argoReconciler.ensureApplicationDeleted(ctx, remoteCentralNamespace) - if err != nil { - return nil, errors.Wrapf(err, "unable to delete ArgoCD application for central %s/%s", remoteCentral.Metadata.Namespace, remoteCentral.Metadata.Name) - } - if !ok { - return nil, errors.New("ArgoCD application not yet deleted") - } - } + if err := r.argoReconciler.ensureApplicationExists(ctx, remoteCentral, centralDBConnectionString); err != nil { + return nil, errors.Wrapf(err, "unable to install ArgoCD application for central %s/%s", remoteCentral.Metadata.Namespace, remoteCentral.Metadata.Name) } if err = r.reconcileDeclarativeConfigurationData(ctx, remoteCentral); err != nil { @@ -321,34 +306,7 @@ func (r *CentralReconciler) Reconcile(ctx context.Context, remoteCentral private return status, nil } -func isArgoCdEnabledForTenant(remoteCentral private.ManagedCentral) bool { - tenantResourceValues := remoteCentral.Spec.TenantResourcesValues - if tenantResourceValues == nil { - return false - } - argoCdIntf, ok := tenantResourceValues["argoCd"] - if !ok { - return false - } - argoCd, ok := argoCdIntf.(map[string]interface{}) - if !ok { - return false - } - enabled, ok := argoCd["enabled"] - if !ok { - return false - } - enabledBool, ok := enabled.(bool) - if !ok { - return false - } - return enabledBool -} - func isArgoCdCentralEnabledForTenant(remoteCentral private.ManagedCentral) bool { - if !isArgoCdEnabledForTenant(remoteCentral) { - return false - } tenantResourceValues := remoteCentral.Spec.TenantResourcesValues if tenantResourceValues == nil { return false