fix(network policies) reviewer comment

Signed-off-by: Simon Guyennet <simon.guyennet@corp.ovh.com>
goharbor · Feb 19, 2021 · e73fbd8 · e73fbd8
1 parent 6e9fede
commit e73fbd8
Show file tree

Hide file tree

Showing 37 changed files with 296 additions and 218 deletions.
diff --git a/Makefile b/Makefile
@@ -106,7 +106,7 @@ diff:
 	git status
 	git diff --stat --diff-filter=d --exit-code HEAD
 
-GO_TEST_OPTS ?= -vet=off
+GO_TEST_OPTS ?= -p 1 -vet=off
 
 .PHONY: go-test
 go-test: install
@@ -448,7 +448,7 @@ certmanager: helm jetstack
 	$(HELM) repo add jetstack https://charts.jetstack.io # https://cert-manager.io/docs/installation/kubernetes/
 	$(HELM) upgrade --install certmanager jetstack/cert-manager \
 		--namespace $(CERTMANAGER_NAMESPACE) \
-		--version v0.15.1 \
+		--version v1.0.3 \
 		--set installCRDs=true
 
 .PHONY: jetstack

diff --git a/apis/meta/v1alpha1/network_policies.go b/apis/meta/v1alpha1/network_policies.go
@@ -0,0 +1,7 @@
+package v1alpha1
+
+const (
+	NetworkPoliciesAnnotationName     = "goharbor.io/network-policies"
+	NetworkPoliciesAnnotationEnabled  = "true"
+	NetworkPoliciesAnnotationDisabled = "false"
+)
diff --git a/charts/harbor-operator/templates/clusterrole.yaml b/charts/harbor-operator/templates/clusterrole.yaml
@@ -274,4 +274,16 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+  - networking.k8s.io
+  resources:
+  - networkpolicies
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
 {{- end -}}
diff --git a/controllers/goharbor/chartmuseum/networkpolicies.go b/controllers/goharbor/chartmuseum/networkpolicies.go
@@ -14,6 +14,24 @@ import (
 
 type NetworkPolicy graph.Resource
 
+func (r *Reconciler) AddNetworkPolicies(ctx context.Context, chartMuseum *goharborv1alpha2.ChartMuseum) error {
+	areNetworkPoliciesEnabled, err := r.AreNetworkPoliciesEnabled(ctx, chartMuseum)
+	if err != nil {
+		return errors.Wrapf(err, "cannot get status")
+	}
+
+	if !areNetworkPoliciesEnabled {
+		return nil
+	}
+
+	_, err = r.AddIngressNetworkPolicy(ctx, chartMuseum)
+	if err != nil {
+		return errors.Wrapf(err, "ingress")
+	}
+
+	return nil
+}
+
 func (r *Reconciler) AddIngressNetworkPolicy(ctx context.Context, chartmuseum *goharborv1alpha2.ChartMuseum) (NetworkPolicy, error) {
 	networkPolicy, err := r.GetIngressNetworkPolicy(ctx, chartmuseum)
 	if err != nil {
@@ -26,8 +44,13 @@ func (r *Reconciler) AddIngressNetworkPolicy(ctx context.Context, chartmuseum *g
 }
 
 func (r *Reconciler) GetIngressNetworkPolicy(ctx context.Context, chartmuseum *goharborv1alpha2.ChartMuseum) (*netv1.NetworkPolicy, error) {
-	httpPort := intstr.FromString(harbormetav1.ChartMuseumHTTPPortName)
-	httpsPort := intstr.FromString(harbormetav1.ChartMuseumHTTPSPortName)
+	var port intstr.IntOrString
+
+	if chartmuseum.Spec.Server.TLS != nil {
+		port = intstr.FromString(harbormetav1.ChartMuseumHTTPSPortName)
+	} else {
+		port = intstr.FromString(harbormetav1.ChartMuseumHTTPPortName)
+	}
 
 	return &netv1.NetworkPolicy{
 		ObjectMeta: metav1.ObjectMeta{
@@ -37,14 +60,9 @@ func (r *Reconciler) GetIngressNetworkPolicy(ctx context.Context, chartmuseum *g
 		Spec: netv1.NetworkPolicySpec{
 			Ingress: []netv1.NetworkPolicyIngressRule{
 				{
-					Ports: []netv1.NetworkPolicyPort{
-						{
-							Port: &httpPort,
-						},
-						{
-							Port: &httpsPort,
-						},
-					},
+					Ports: []netv1.NetworkPolicyPort{{
+						Port: &port,
+					}},
 				},
 			},
 			PodSelector: metav1.LabelSelector{

diff --git a/controllers/goharbor/chartmuseum/resources.go b/controllers/goharbor/chartmuseum/resources.go
@@ -49,17 +49,7 @@ func (r *Reconciler) AddResources(ctx context.Context, resource resources.Resour
 		return errors.Wrapf(err, "cannot add deployment %s", deployment.GetName())
 	}
 
-	areNetworkPoliciesEnabled, err := r.AreNetworkPoliciesEnabled(ctx, chartMuseum)
-	if err != nil {
-		return errors.Wrapf(err, "cannot get network policies status")
-	}
-
-	if areNetworkPoliciesEnabled {
-		_, err = r.AddIngressNetworkPolicy(ctx, chartMuseum)
-		if err != nil {
-			return errors.Wrapf(err, "ingress network policy")
-		}
-	}
+	err = r.AddNetworkPolicies(ctx, chartMuseum)
 
-	return nil
+	return errors.Wrap(err, "network policies")
 }
diff --git a/controllers/goharbor/core/networkpolicies.go b/controllers/goharbor/core/networkpolicies.go
@@ -14,6 +14,24 @@ import (
 
 type NetworkPolicy graph.Resource
 
+func (r *Reconciler) AddNetworkPolicies(ctx context.Context, core *goharborv1alpha2.Core) error {
+	areNetworkPoliciesEnabled, err := r.AreNetworkPoliciesEnabled(ctx, core)
+	if err != nil {
+		return errors.Wrapf(err, "cannot get status")
+	}
+
+	if !areNetworkPoliciesEnabled {
+		return nil
+	}
+
+	_, err = r.AddIngressNetworkPolicy(ctx, core)
+	if err != nil {
+		return errors.Wrapf(err, "ingress")
+	}
+
+	return nil
+}
+
 func (r *Reconciler) AddIngressNetworkPolicy(ctx context.Context, core *goharborv1alpha2.Core) (NetworkPolicy, error) {
 	networkPolicy, err := r.GetIngressNetworkPolicy(ctx, core)
 	if err != nil {
@@ -26,8 +44,13 @@ func (r *Reconciler) AddIngressNetworkPolicy(ctx context.Context, core *goharbor
 }
 
 func (r *Reconciler) GetIngressNetworkPolicy(ctx context.Context, core *goharborv1alpha2.Core) (*netv1.NetworkPolicy, error) {
-	httpPort := intstr.FromString(harbormetav1.CoreHTTPPortName)
-	httpsPort := intstr.FromString(harbormetav1.CoreHTTPSPortName)
+	var port intstr.IntOrString
+
+	if core.Spec.Components.TLS != nil {
+		port = intstr.FromString(harbormetav1.CoreHTTPSPortName)
+	} else {
+		port = intstr.FromString(harbormetav1.CoreHTTPPortName)
+	}
 
 	return &netv1.NetworkPolicy{
 		ObjectMeta: metav1.ObjectMeta{
@@ -37,14 +60,9 @@ func (r *Reconciler) GetIngressNetworkPolicy(ctx context.Context, core *goharbor
 		Spec: netv1.NetworkPolicySpec{
 			Ingress: []netv1.NetworkPolicyIngressRule{
 				{
-					Ports: []netv1.NetworkPolicyPort{
-						{
-							Port: &httpPort,
-						},
-						{
-							Port: &httpsPort,
-						},
-					},
+					Ports: []netv1.NetworkPolicyPort{{
+						Port: &port,
+					}},
 				},
 			},
 			PodSelector: metav1.LabelSelector{

diff --git a/controllers/goharbor/core/resources.go b/controllers/goharbor/core/resources.go
@@ -59,17 +59,7 @@ func (r *Reconciler) AddResources(ctx context.Context, resource resources.Resour
 		return errors.Wrapf(err, "cannot add deployment %s", deployment.GetName())
 	}
 
-	areNetworkPoliciesEnabled, err := r.AreNetworkPoliciesEnabled(ctx, core)
-	if err != nil {
-		return errors.Wrapf(err, "cannot get network policies status")
-	}
-
-	if areNetworkPoliciesEnabled {
-		_, err = r.AddIngressNetworkPolicy(ctx, core)
-		if err != nil {
-			return errors.Wrapf(err, "ingress network policy")
-		}
-	}
+	err = r.AddNetworkPolicies(ctx, core)
 
-	return nil
+	return errors.Wrap(err, "network policies")
 }
diff --git a/controllers/goharbor/harbor/chartmuseum.go b/controllers/goharbor/harbor/chartmuseum.go
@@ -62,7 +62,7 @@ func (r *Reconciler) AddChartMuseum(ctx context.Context, harbor *goharborv1alpha
 	return ChartMuseum(chartmuseumRes), errors.Wrap(err, "add")
 }
 
-func (r *Reconciler) GetChartMuseum(ctx context.Context, harbor *goharborv1alpha2.Harbor) (*goharborv1alpha2.ChartMuseum, error) {
+func (r *Reconciler) GetChartMuseum(ctx context.Context, harbor *goharborv1alpha2.Harbor) (*goharborv1alpha2.ChartMuseum, error) { //nolint:funlen
 	name := r.NormalizeName(ctx, harbor.GetName())
 	namespace := harbor.GetNamespace()
 
@@ -89,9 +89,11 @@ func (r *Reconciler) GetChartMuseum(ctx context.Context, harbor *goharborv1alpha
 
 	return &goharborv1alpha2.ChartMuseum{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        name,
-			Namespace:   namespace,
-			Annotations: r.NetworkPolicyAnnotationDisabled(),
+			Name:      name,
+			Namespace: namespace,
+			Annotations: map[string]string{
+				harbormetav1.NetworkPoliciesAnnotationName: harbormetav1.NetworkPoliciesAnnotationDisabled,
+			},
 		},
 		Spec: goharborv1alpha2.ChartMuseumSpec{
 			ComponentSpec: harbor.Spec.ChartMuseum.ComponentSpec,

diff --git a/controllers/goharbor/harbor/core.go b/controllers/goharbor/harbor/core.go
@@ -410,9 +410,11 @@ func (r *Reconciler) GetCore(ctx context.Context, harbor *goharborv1alpha2.Harbo
 
 	return &goharborv1alpha2.Core{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        name,
-			Namespace:   namespace,
-			Annotations: r.NetworkPolicyAnnotationDisabled(),
+			Name:      name,
+			Namespace: namespace,
+			Annotations: map[string]string{
+				harbormetav1.NetworkPoliciesAnnotationName: harbormetav1.NetworkPoliciesAnnotationDisabled,
+			},
 		},
 		Spec: goharborv1alpha2.CoreSpec{
 			ComponentSpec: harbor.Spec.Registry.ComponentSpec,

diff --git a/controllers/goharbor/harbor/harbor.go b/controllers/goharbor/harbor/harbor.go
@@ -31,6 +31,7 @@ type Reconciler struct {
 // +kubebuilder:rbac:groups=goharbor.io,resources=harbors/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=goharbor.io,resources=chartmuseums;cores;jobservices;notaryservers;notarysigners;portals;registries;registrycontrollers;trivies,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=get;list;watch;create;update;patch;delete
+// +kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=cert-manager.io,resources=issuers;certificates,verbs=get;list;watch;create;update;patch;delete
 
 func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {

diff --git a/controllers/goharbor/harbor/jobservice.go b/controllers/goharbor/harbor/jobservice.go
@@ -147,9 +147,11 @@ func (r *Reconciler) GetJobService(ctx context.Context, harbor *goharborv1alpha2
 
 	return &goharborv1alpha2.JobService{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        name,
-			Namespace:   namespace,
-			Annotations: r.NetworkPolicyAnnotationDisabled(),
+			Name:      name,
+			Namespace: namespace,
+			Annotations: map[string]string{
+				harbormetav1.NetworkPoliciesAnnotationName: harbormetav1.NetworkPoliciesAnnotationDisabled,
+			},
 		},
 		Spec: goharborv1alpha2.JobServiceSpec{
 			ComponentSpec: harbor.Spec.Registry.ComponentSpec,

diff --git a/controllers/goharbor/harbor/networkpolicies.go b/controllers/goharbor/harbor/networkpolicies.go
@@ -121,8 +121,13 @@ func (r *Reconciler) AddCoreIngressNetworkPolicy(ctx context.Context, harbor *go
 }
 
 func (r *Reconciler) GetCoreIngressNetworkPolicy(ctx context.Context, harbor *goharborv1alpha2.Harbor) (*netv1.NetworkPolicy, error) { //nolint:dupl
-	httpPort := intstr.FromString(harbormetav1.CoreHTTPPortName)
-	httpsPort := intstr.FromString(harbormetav1.CoreHTTPSPortName)
+	var port intstr.IntOrString
+
+	if harbor.Spec.Expose.Core.TLS != nil {
+		port = intstr.FromString(harbormetav1.CoreHTTPSPortName)
+	} else {
+		port = intstr.FromString(harbormetav1.CoreHTTPPortName)
+	}
 
 	return &netv1.NetworkPolicy{
 		ObjectMeta: metav1.ObjectMeta{
@@ -132,14 +137,9 @@ func (r *Reconciler) GetCoreIngressNetworkPolicy(ctx context.Context, harbor *go
 		Spec: netv1.NetworkPolicySpec{
 			Ingress: []netv1.NetworkPolicyIngressRule{
 				{
-					Ports: []netv1.NetworkPolicyPort{
-						{
-							Port: &httpPort,
-						},
-						{
-							Port: &httpsPort,
-						},
-					},
+					Ports: []netv1.NetworkPolicyPort{{
+						Port: &port,
+					}},
 				},
 			},
 			PodSelector: metav1.LabelSelector{

diff --git a/controllers/goharbor/harbor/notaryserver.go b/controllers/goharbor/harbor/notaryserver.go
@@ -194,9 +194,11 @@ func (r *Reconciler) GetNotaryServer(ctx context.Context, harbor *goharborv1alph
 
 	return &goharborv1alpha2.NotaryServer{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        name,
-			Namespace:   namespace,
-			Annotations: r.NetworkPolicyAnnotationDisabled(),
+			Name:      name,
+			Namespace: namespace,
+			Annotations: map[string]string{
+				harbormetav1.NetworkPoliciesAnnotationName: harbormetav1.NetworkPoliciesAnnotationDisabled,
+			},
 		},
 		Spec: goharborv1alpha2.NotaryServerSpec{
 			ComponentSpec: harbor.Spec.Notary.Server,

diff --git a/controllers/goharbor/harbor/notarysigner.go b/controllers/goharbor/harbor/notarysigner.go
@@ -339,9 +339,11 @@ func (r *Reconciler) GetNotarySigner(ctx context.Context, harbor *goharborv1alph
 
 	return &goharborv1alpha2.NotarySigner{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        name,
-			Namespace:   namespace,
-			Annotations: r.NetworkPolicyAnnotationDisabled(),
+			Name:      name,
+			Namespace: namespace,
+			Annotations: map[string]string{
+				harbormetav1.NetworkPoliciesAnnotationName: harbormetav1.NetworkPoliciesAnnotationDisabled,
+			},
 		},
 		Spec: goharborv1alpha2.NotarySignerSpec{
 			ComponentSpec: harbor.Spec.Notary.Signer,

diff --git a/controllers/goharbor/harbor/portal.go b/controllers/goharbor/harbor/portal.go
@@ -52,9 +52,11 @@ func (r *Reconciler) GetPortal(ctx context.Context, harbor *goharborv1alpha2.Har
 
 	return &goharborv1alpha2.Portal{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        name,
-			Namespace:   namespace,
-			Annotations: r.NetworkPolicyAnnotationDisabled(),
+			Name:      name,
+			Namespace: namespace,
+			Annotations: map[string]string{
+				harbormetav1.NetworkPoliciesAnnotationName: harbormetav1.NetworkPoliciesAnnotationDisabled,
+			},
 		},
 		Spec: goharborv1alpha2.PortalSpec{
 			ComponentSpec: harbor.Spec.Portal,

diff --git a/controllers/goharbor/harbor/registry.go b/controllers/goharbor/harbor/registry.go
@@ -206,9 +206,11 @@ func (r *Reconciler) GetRegistry(ctx context.Context, harbor *goharborv1alpha2.H
 
 	return &goharborv1alpha2.Registry{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        name,
-			Namespace:   namespace,
-			Annotations: r.NetworkPolicyAnnotationDisabled(),
+			Name:      name,
+			Namespace: namespace,
+			Annotations: map[string]string{
+				harbormetav1.NetworkPoliciesAnnotationName: harbormetav1.NetworkPoliciesAnnotationDisabled,
+			},
 		},
 		Spec: goharborv1alpha2.RegistrySpec{
 			ComponentSpec: harbor.Spec.Registry.ComponentSpec,

diff --git a/controllers/goharbor/harbor/registryctl.go b/controllers/goharbor/harbor/registryctl.go
@@ -61,9 +61,11 @@ func (r *Reconciler) GetRegistryCtl(ctx context.Context, harbor *goharborv1alpha
 
 	return &goharborv1alpha2.RegistryController{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        name,
-			Namespace:   namespace,
-			Annotations: r.NetworkPolicyAnnotationDisabled(),
+			Name:      name,
+			Namespace: namespace,
+			Annotations: map[string]string{
+				harbormetav1.NetworkPoliciesAnnotationName: harbormetav1.NetworkPoliciesAnnotationDisabled,
+			},
 		},
 		Spec: goharborv1alpha2.RegistryControllerSpec{
 			ComponentSpec: harbor.Spec.Registry.ComponentSpec,

diff --git a/controllers/goharbor/harbor/trivy.go b/controllers/goharbor/harbor/trivy.go
@@ -128,9 +128,11 @@ func (r *Reconciler) GetTrivy(ctx context.Context, harbor *goharborv1alpha2.Harb
 
 	return &goharborv1alpha2.Trivy{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        name,
-			Namespace:   namespace,
-			Annotations: r.NetworkPolicyAnnotationDisabled(),
+			Name:      name,
+			Namespace: namespace,
+			Annotations: map[string]string{
+				harbormetav1.NetworkPoliciesAnnotationName: harbormetav1.NetworkPoliciesAnnotationDisabled,
+			},
 		},
 		Spec: goharborv1alpha2.TrivySpec{
 			ComponentSpec: harbor.Spec.Trivy.ComponentSpec,