Refactor (#488)

* Refactor * started work on processor and reconciliation packages * finish processor * finished initial namespace controller refactor, started routing controller * finish rough refactor of controllers * Fix schemas, make it build and run * fix namespace controller so tests pass * adding more tests, fixing errors * Fixing tests * more refactoring * Fix deployment patching and more tests * fix custom certs * fix ingress and watched tests, cloudsql proxy proper security context * Fix routing * skipjob testing * all tests fixed except condition/status * remove istio pkg from gitignore * clean up skipjobs a bit, add schemas for each controller * Add SKIPObject and SkiperatorStatus * Fix tests, fix status (not conditions) * Clean up status stuff a bit * fix(ish) skipjob conditions * Add reason to error state function * remove unused files, add some todos * only use app label on certain resources. improve tests * fix skiperator ignore on resources and add unit tests to makefile * remove unneccessary pointers * add warning event for mistyped resource labels * Add missing namespace creation to setup-local * added alloy support in netpol/default deny * Fix panic when logging * review * review * more review * multigenerator: simplify resource generation code and expandability (#503) Co-authored-by: Martin Haram Nygård <[email protected]> * review * fix test * review, moved commonspec to types, some changes to status * more review corrections * Fix status reconciles, tweak status on pending * fix test * fix typo, add todos * fix all tests failing, add dir for cluster-config * upgrade docker pkg for pharos scan * add resourceDiff struct for more readable diffs * add comment for image pull secret * Dynamic port allocation (#506) * add support for dynamic port * rebase * fix some tests * fix validation and tests * adjust tests to be more resilient * dont add netpol rules that doesn't have a port * add todo * add todo * custom-cert: bugfix * fix missing rbac permissions --------- Co-authored-by: Eline Henriksen <[email protected]> Co-authored-by: Bård Ove Hoel <[email protected]> Co-authored-by: Eline Henriksen <[email protected]> Co-authored-by: Even Holthe <[email protected]> Co-authored-by: Even Holthe <[email protected]>
kartverket · Aug 14, 2024 · f041aab · f041aab
1 parent 843d670
commit f041aab
Show file tree

Hide file tree

Showing 176 changed files with 6,788 additions and 5,291 deletions.
diff --git a/.gitignore b/.gitignore
@@ -7,4 +7,5 @@
 .vscode/
 .idea/
 dist/
-istio*
+istio*
+!**/istio/
diff --git a/Makefile b/Makefile
@@ -95,7 +95,7 @@ install-digdirator-crds:
 install-skiperator: generate
 	@kubectl create namespace skiperator-system --context $(SKIPERATOR_CONTEXT) || true
 	@kubectl apply -f config/ --recursive --context $(SKIPERATOR_CONTEXT)
-	@kubectl apply -f samples/ --recursive --context $(SKIPERATOR_CONTEXT) || true
+	@kubectl apply -f tests/cluster-config/ --recursive --context $(SKIPERATOR_CONTEXT) || true
 
 .PHONY: install-test-tools
 install-test-tools:
@@ -112,6 +112,15 @@ test: install-test-tools install-skiperator
 	@./bin/chainsaw test --kube-context $(SKIPERATOR_CONTEXT) --config tests/config.yaml --test-dir tests/ && \
     echo "Test succeeded" || (echo "Test failed" && exit 1)
 
+.PHONY: run-unit-tests
+run-unit-tests:
+	@failed_tests=$$(go test ./... 2>&1 | grep "^FAIL" | awk '{print $$2}'); \
+		if [ -n "$$failed_tests" ]; then \
+			echo -e "\033[31mFailed Unit Tests: [$$failed_tests]\033[0m" && exit 1; \
+		else \
+			echo -e "\033[32mAll unit tests passed\033[0m"; \
+		fi
+
 .PHONY: run-test
 run-test: build
 	@echo "Starting skiperator in background..."
@@ -127,4 +136,4 @@ run-test: build
 			$(MAKE) test-single dir=$(TEST_DIR); \
 		fi; \
 	) && \
-	(echo "Stopping skiperator (PID $$PID)..." && kill $$PID)  || (echo "Test or skiperator failed. Stopping skiperator (PID $$PID)" && kill $$PID && exit 1)
+	(echo "Stopping skiperator (PID $$PID)..." && kill $$PID && echo "running unit tests..." && $(MAKE) run-unit-tests)  || (echo "Test or skiperator failed. Stopping skiperator (PID $$PID)" && kill $$PID && exit 1)
diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go
@@ -3,15 +3,13 @@ package v1alpha1
 import (
 	"encoding/json"
 	"errors"
-	"time"
-
 	"github.com/kartverket/skiperator/api/v1alpha1/digdirator"
 	"github.com/kartverket/skiperator/api/v1alpha1/podtypes"
-	"golang.org/x/exp/slices"
 	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	"time"
 )
 
 // +kubebuilder:object:root=true
@@ -30,14 +28,13 @@ type ApplicationList struct {
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:resource:shortName="app"
-// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.application.status`
+// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.summary.status`
 type Application struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
-	Spec ApplicationSpec `json:"spec,omitempty"`
-
-	Status ApplicationStatus `json:"status,omitempty"`
+	Spec   ApplicationSpec  `json:"spec,omitempty"`
+	Status SkiperatorStatus `json:"status,omitempty"`
 }
 
 // +kubebuilder:object:generate=true
@@ -68,7 +65,6 @@ type ApplicationSpec struct {
 	// Ingresses must be lowercase, contain no spaces, be a non-empty string, and have a hostname/domain separated by a period
 	// They can optionally be suffixed with a plus and name of a custom TLS secret located in the istio-gateways namespace.
 	// E.g. "foo.atkv3-dev.kartverket-intern.cloud+env-wildcard-cert"
-	//
 	//+kubebuilder:validation:Optional
 	Ingresses []string `json:"ingresses,omitempty"`
 
@@ -329,38 +325,6 @@ type PrometheusConfig struct {
 	AllowAllMetrics bool `json:"allowAllMetrics,omitempty"`
 }
 
-// ApplicationStatus
-//
-// A status field shown on the Application resource which contains information regarding all controllers present on the Application.
-// Will for example show errors on the Deployment field when something went wrong when attempting to create a Deployment.
-//
-// +kubebuilder:object:generate=true
-type ApplicationStatus struct {
-	ApplicationStatus Status            `json:"application"`
-	ControllersStatus map[string]Status `json:"controllers"`
-}
-
-// Status
-//
-// +kubebuilder:object:generate=true
-type Status struct {
-	// +kubebuilder:default="Synced"
-	Status StatusNames `json:"status"`
-	// +kubebuilder:default="hello"
-	Message string `json:"message"`
-	// +kubebuilder:default="hello"
-	TimeStamp string `json:"timestamp"`
-}
-
-type StatusNames string
-
-const (
-	SYNCED      StatusNames = "Synced"
-	PROGRESSING StatusNames = "Progressing"
-	ERROR       StatusNames = "Error"
-	PENDING     StatusNames = "Pending"
-)
-
 func NewDefaultReplicas() Replicas {
 	return Replicas{
 		Min:                  2,
@@ -418,106 +382,65 @@ func (a *Application) FillDefaultsSpec() {
 }
 
 func (a *Application) FillDefaultsStatus() {
-	if a.Status.ApplicationStatus.Status == "" {
-		a.Status.ApplicationStatus = Status{
-			Status:    PENDING,
-			Message:   "Default application status, application has not initialized yet",
-			TimeStamp: time.Now().String(),
-		}
-	}
+	var msg string
 
-	if a.Status.ControllersStatus == nil {
-		a.Status.ControllersStatus = make(map[string]Status)
+	if a.Status.Summary.Status == "" {
+		msg = "Default Application status, it has not initialized yet"
+	} else {
+		msg = "Application is trying to reconcile"
 	}
-}
 
-func (a *Application) UpdateApplicationStatus() {
-	newApplicationStatus := a.CalculateApplicationStatus()
-	if newApplicationStatus.Status == a.Status.ApplicationStatus.Status {
-		return
+	a.Status.Summary = Status{
+		Status:    PENDING,
+		Message:   msg,
+		TimeStamp: time.Now().String(),
 	}
 
-	a.Status.ApplicationStatus = newApplicationStatus
-}
-
-func (a *Application) UpdateControllerStatus(controllerName string, message string, status StatusNames) {
-	if a.Status.ControllersStatus[controllerName].Status == status {
-		return
+	if a.Status.SubResources == nil {
+		a.Status.SubResources = make(map[string]Status)
 	}
 
-	newStatus := Status{
-		Status:    status,
-		Message:   message,
-		TimeStamp: time.Now().String(),
+	if len(a.Status.Conditions) == 0 {
+		a.Status.Conditions = make([]metav1.Condition, 0)
 	}
-	a.Status.ControllersStatus[controllerName] = newStatus
-
-	a.UpdateApplicationStatus()
-
 }
 
-func (a *Application) ShouldUpdateApplicationStatus(newStatus Status) bool {
-	shouldUpdate := newStatus.Status != a.Status.ApplicationStatus.Status
-
-	return shouldUpdate
+func (a *Application) GetStatus() *SkiperatorStatus {
+	return &a.Status
 }
 
-func (a *Application) CalculateApplicationStatus() Status {
-	returnStatus := Status{
-		Status:    ERROR,
-		Message:   "CALCULATION DEFAULT, YOU SHOULD NOT SEE THIS MESSAGE. PLEASE LET SKIP KNOW IF THIS MESSAGE IS VISIBLE",
-		TimeStamp: time.Now().String(),
-	}
-	statusList := []string{}
-	for _, s := range a.Status.ControllersStatus {
-		statusList = append(statusList, string(s.Status))
-	}
-
-	if slices.IndexFunc(statusList, func(s string) bool { return s == string(ERROR) }) != -1 {
-		returnStatus.Status = ERROR
-		returnStatus.Message = "One of the controllers is in a failed state"
-		return returnStatus
-	}
-
-	if slices.IndexFunc(statusList, func(s string) bool { return s == string(PROGRESSING) }) != -1 {
-		returnStatus.Status = PROGRESSING
-		returnStatus.Message = "One of the controllers is progressing"
-		return returnStatus
-	}
+func (a *Application) SetStatus(status SkiperatorStatus) {
+	a.Status = status
+}
 
-	if allSameStatus(statusList) {
-		returnStatus.Status = StatusNames(statusList[0])
-		if returnStatus.Status == SYNCED {
-			returnStatus.Message = "All controllers synced"
-		} else if returnStatus.Status == PENDING {
-			returnStatus.Message = "All controllers pending"
-		}
-		return returnStatus
+// TODO clean up labels
+func (a *Application) GetDefaultLabels() map[string]string {
+	return map[string]string{
+		"app.kubernetes.io/managed-by":            "skiperator",
+		"skiperator.kartverket.no/controller":     "application",
+		"application.skiperator.no/app":           a.Name,
+		"application.skiperator.no/app-name":      a.Name,
+		"application.skiperator.no/app-namespace": a.Namespace,
 	}
-
-	return returnStatus
 }
 
-func allSameStatus(a []string) bool {
-	for _, v := range a {
-		if v != a[0] {
-			return false
-		}
+func (a *Application) GetCommonSpec() *CommonSpec {
+	return &CommonSpec{
+		GCP:          a.Spec.GCP,
+		AccessPolicy: a.Spec.AccessPolicy,
 	}
-	return true
 }
 
-func (s *ApplicationSpec) Hosts() ([]Host, error) {
-	var hosts []Host
+func (s *ApplicationSpec) Hosts() (HostCollection, error) {
+	hosts := NewCollection()
+
 	var errorsFound []error
 	for _, ingress := range s.Ingresses {
-		h, err := NewHost(ingress)
+		err := hosts.Add(ingress)
 		if err != nil {
 			errorsFound = append(errorsFound, err)
 			continue
 		}
-
-		hosts = append(hosts, *h)
 	}
 
 	return hosts, errors.Join(errorsFound...)

diff --git a/api/v1alpha1/hosts.go b/api/v1alpha1/hosts.go
@@ -15,6 +15,10 @@ type Host struct {
 	CustomCertificateSecret *string
 }
 
+type HostCollection struct {
+	hosts map[string]*Host
+}
+
 func NewHost(hostname string) (*Host, error) {
 	if len(hostname) == 0 {
 		return nil, fmt.Errorf("hostname cannot be empty")
@@ -52,3 +56,52 @@ func NewHost(hostname string) (*Host, error) {
 func (h *Host) UsesCustomCert() bool {
 	return h.CustomCertificateSecret != nil
 }
+
+func NewCollection() HostCollection {
+	return HostCollection{
+		hosts: map[string]*Host{},
+	}
+}
+
+func (hs *HostCollection) Add(hostname string) error {
+	h, err := NewHost(hostname)
+	if err != nil {
+		return err
+	}
+
+	existingValue, alreadyPresent := hs.hosts[h.Hostname]
+
+	switch alreadyPresent {
+	case true:
+		if existingValue.UsesCustomCert() {
+			return fmt.Errorf("host '%s' is already defined and using a custom certificate", existingValue.Hostname)
+		}
+		fallthrough
+	case false:
+		fallthrough
+	default:
+		hs.hosts[h.Hostname] = h
+	}
+
+	return nil
+}
+
+func (hs *HostCollection) AllHosts() []*Host {
+	hosts := make([]*Host, 0, len(hs.hosts))
+	for _, host := range hs.hosts {
+		hosts = append(hosts, host)
+	}
+	return hosts
+}
+
+func (hs *HostCollection) Hostnames() []string {
+	hostnames := make([]string, 0, len(hs.hosts))
+	for hostname := range hs.hosts {
+		hostnames = append(hostnames, hostname)
+	}
+	return hostnames
+}
+
+func (hs *HostCollection) Count() int {
+	return len(hs.hosts)
+}
diff --git a/api/v1alpha1/podtypes/access_policy.go b/api/v1alpha1/podtypes/access_policy.go
@@ -1,5 +1,7 @@
 package podtypes
 
+import v1 "k8s.io/api/networking/v1"
+
 // AccessPolicy
 //
 // Zero trust dictates that only applications with a reason for being able
@@ -19,7 +21,7 @@ type AccessPolicy struct {
 	// internet is the Application allowed to send requests to?
 	//
 	//+kubebuilder:validation:Optional
-	Outbound OutboundPolicy `json:"outbound,omitempty"`
+	Outbound *OutboundPolicy `json:"outbound,omitempty"`
 }
 
 // InboundPolicy
@@ -76,6 +78,9 @@ type InternalRule struct {
 	//
 	//+kubebuilder:validation:Optional
 	NamespacesByLabel map[string]string `json:"namespacesByLabel,omitempty"`
+	// The ports to allow for the above application.
+	//+kubebuilder:validation:Optional
+	Ports []v1.NetworkPolicyPort `json:"ports,omitempty"`
 }
 
 // ExternalRule