Merge branch 'master' into json-fields

.github/workflows/test.yml

@@ -30,6 +30,11 @@ jobs:
         go-version: [1.14.x]
         os: [ubuntu-latest, windows-latest]
     runs-on: ${{ matrix.os }}
+    env:
+      AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
+      AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
+      AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }}
+      AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
     steps:
       - name: Install Go
         uses: actions/setup-go@v1
@@ -45,6 +50,12 @@ jobs:
       - name: Run generator CI (Linux)
         if: ${{ runner.os == 'Linux' }} 
         run: make -C ./hack/generator ci
+      - name: Make generated controller integration tests (and unit tests)
+        if: ${{ runner.os == 'Linux' && env.AZURE_TENANT_ID != '' }}
+        run: make -C ./hack/generated test-int-cover
+      - name: Make generated controller test
+        if: ${{ runner.os == 'Linux' && env.AZURE_TENANT_ID == '' }}
+        run: make -C ./hack/generated test-cover
       - name: Test generator (Windows)
         if: ${{ runner.os == 'Windows' }}
         # Makefile not supported. Linux will run lints so

Makefile

@@ -27,12 +27,12 @@ test test-int test-cover test-cover-int: export TEST_ASSET_ETCD = $(ETCD)
 test: $(KUBECTL) $(KUBE_APISERVER) $(ETCD) lint header-check ## Run tests
 	$(GO) test -v ./...
 
-test-int: .env $(KUBECTL) $(KUBE_APISERVER) $(ETCD) header-check lint ## Run integration tests
+test-int: $(ROOT_DIR)/.env $(KUBECTL) $(KUBE_APISERVER) $(ETCD) header-check lint ## Run integration tests
 	# MUST be executed as single command, or env vars will not propagate to test execution
 	. .env && $(GO) test -v ./... -tags integration
 
 .env: ## create a service principal and save the identity to .env for use in integration tests (requries jq and az)
-	./scripts/create_testing_creds.sh
+	$(SCRIPTS_DIR)/create_testing_creds.sh
 
 test-cover: $(KUBECTL) $(KUBE_APISERVER) $(ETCD) header-check lint ## Run tests w/ code coverage (./cover.out)
 	$(GO) test ./... -coverprofile=cover.out -coverpkg=./...
@@ -95,7 +95,7 @@ generate: manifests $(CONTROLLER_GEN) $(CONVERSION_GEN) ## Generate code
 ## --------------------------------------
 
 .PHONY: tilt-up
-tilt-up: kind-create .env ## start tilt and build kind cluster if needed
+tilt-up: kind-create $(ROOT_DIR)/.env ## start tilt and build kind cluster if needed
 	tilt up
 
 .PHONY: kind-reset

hack/Makefile

@@ -0,0 +1,11 @@
+SHELL           = /bin/bash
+
+V = 0
+Q = $(if $(filter 1,$V),,@)
+
+.PHONY: all
+all:
+	$(Q) cd generator && make all
+	$(Q) cd generated && make all
+
+

hack/generated/Dockerfile

@@ -0,0 +1,36 @@
+# Build the manager binary
+FROM golang:1.13.15 as builder
+
+WORKDIR /workspace
+
+# Copy the Go Modules manifests
+COPY go.mod go.mod
+COPY go.sum go.sum
+# cache deps before building and copying source so that we don't need to re-download as much
+# and so that source changes don't invalidate our downloaded layer
+RUN go mod download
+
+# Copy the go source
+# COPY . ./
+COPY main.go main.go
+COPY apis/ apis/
+COPY controllers/ controllers/
+COPY pkg/ pkg/
+
+# Build
+# TODO: Use Makefile here -- right now it's awkward to do so because:
+#   1. tools.mk is required for the makefile from the above directory, but Dockerfile can only look in its directory and below.
+#   2. Having Dockerfile here but building it from above could work except that there's another Dockerfile and a .dockerignore
+#      up above that break things. For now we just build by hand
+# RUN make build
+
+# TODO: Do we want CGO_ENALBED=0 and the other options below in the makefile?
+RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o k8sinfra-controller main.go
+
+# Use distroless as minimal base image to package the manager binary
+# Refer to https://github.com/GoogleContainerTools/distroless for more details
+FROM gcr.io/distroless/static:nonroot
+WORKDIR /
+COPY --from=builder /workspace/k8sinfra-controller .
+USER nonroot:nonroot
+ENTRYPOINT ["/k8sinfra-controller"]

hack/generated/Makefile

@@ -2,9 +2,8 @@ SHELL             = /bin/bash
 PACKAGE           = github.com/Azure/k8s-infra/hack/generated
 APP               = k8sinfra-controller
 
-timestamp         := $(shell /bin/date "+%Y%m%d-%H%M%S")
-# CONFIG_REGISTRY = kind-registry:5000/fake/k8s-infra-controller:latest
-IMG               ?= k8s-infra-generated-contoller:$(timestamp)
+CONFIG_REGISTRY = kind-registry:5000/fake/k8s-infra-controller:latest
+IMG               ?= k8s-infra-generated-contoller:latest
 KIND_CLUSTER_NAME = k8sinfra-generated
 
 include ../../tools.mk
@@ -13,18 +12,23 @@ CRD_OPTIONS     ?= "crd:crdVersions=v1,allowDangerousTypes=true"
 GO_DIRS         := $(shell $(GO) list -f '{{.Dir}}' ./...)
 # We exclude the apis folder because it's really large and test discovery takes a good amount of time (>10s)
 GO_DIRS_TO_TEST := $(shell $(GO) list -f '{{.Dir}}' ./... | grep -v /apis/)
+CONTROLLER_DEBUG_LOGLEVEL := 4
 
 V = 0
 Q = $(if $(filter 1,$V),,@)
 
 .PHONY: all
-all: generate header-check fmt build test
+all: generate header-check fmt build test test-int  # TODO: Do we want to remove test-int from the all target so it's not run every time locally?
 
 # There is a ci specific target because we want the CI pass to fail if
 # the code has not been go fmt-ed, whereas locally we want "make all"
 # to just format the code for you
 .PHONY: ci
-ci: generate build test-cover
+ci: generate build  # test-cover or test-cover-int will be called by the CI job directly
+
+## --------------------------------------
+## Build
+## --------------------------------------
 
 .PHONY: lint
 lint: $(GOLANGCI_LINT) ; $(info $(M) running golangci configured linters…) ## Lint codebase
@@ -42,24 +46,13 @@ fmt: ; $(info $(M) running gofmt…) @ ## Run gofmt on all source files
 tidy: ; $(info $(M) running tidy…) @ ## Run tidy
 	$Q $(GO) mod tidy
 
-.PHONY: test
-test: ; $(info $(M) running go test…)
-	$(Q) $(GO) test $(GO_DIRS_TO_TEST) -tags=noexit
-
-.PHONY: test-cover
-test-cover: $(GCOV2LCOV) ; $(info $(M) running go test…)
-	# NOTE: if you update the 'test-cover' target, also update ./github/workflows/test.yml
-	#       for the Windows part of the "test-generator" job.
-	$(Q) $(GO) test -tags=noexit -race -covermode atomic -coverprofile=cover.out -coverpkg=./... $(GO_DIRS_TO_TEST)
-	$(Q) $(GCOV2LCOV) -infile cover.out -outfile coverage.lcov
-
 .PHONY: generate
 generate: $(CONTROLLER_GEN) $(CONVERSION_GEN) ## Generate code
 	# Force regeneration of all of the conversions
-	@echo "Deleting old controller gen files"
+	@echo "Deleting old deepcopy files"
 	$(Q) find "./apis" -type f -name "zz_generated.*" -delete
 
-	@echo "Executing controller-gen"
+	@echo "Executing controller-gen to generate deepcopy functions"
 	$(Q) $(CONTROLLER_GEN) object:headerFile=../boilerplate.go.txt paths="./..."
 
 #	@echo "Executing conversion-gen"
@@ -70,15 +63,64 @@ generate: $(CONTROLLER_GEN) $(CONVERSION_GEN) ## Generate code
 #		--go-header-file=../boilerplate.go.txt
 
 	# Force regeneration of all of the CRDs
-	@echo "Deleting old CRDs"
+	@echo "Deleting old CRD YAMLs"
 	$(Q) if [ -d "./config/crd/bases" ]; then find "./config/crd/bases" -type f -name "*" -delete; fi
 
-	@echo "Executing controller-gen to generate CRDs"
-	$(Q) $(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./apis/..." output:crd:artifacts:config=config/crd/bases
+	@echo "Executing controller-gen to generate CRD and RBAC YAMLs"
+	$(Q) $(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
 
 .PHONY: build
 build: tidy lint ; $(info $(M) building ./bin/$(APP))
-	$(Q) $(GO) build -o ./bin/$(APP)
+	$(Q) CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on $(GO) build -o ./bin/$(APP)
+
+## --------------------------------------
+## Test
+## --------------------------------------
+.PHONY: test
+test: ; $(info $(M) running go test…)
+	$(Q) $(GO) test $(GO_DIRS_TO_TEST) -short -tags=noexit
+
+.PHONY: test-cover
+test-cover: $(GCOV2LCOV) ; $(info $(M) running go test…)
+	# NOTE: if you update the 'test-cover' target, also update ./github/workflows/test.yml
+	#       for the Windows part of the "test-generator" job.
+	$(Q) $(GO) test -short -tags=noexit -race -covermode atomic -coverprofile=cover.out -coverpkg=./... $(GO_DIRS_TO_TEST)
+	$(Q) $(GCOV2LCOV) -infile cover.out -outfile coverage.lcov
+
+# Initially this target uses kind as there is no need to end to end test on Azure.
+# Eventually when we start supporting features like MSI that only work in Azure,
+# it's likely we'll need to make the integration tests (or at least some of them)
+# run in an AKS cluster.
+.PHONY: test-int-no-cleanup
+test-int-no-cleanup: ; $(info $(M) running controller integration test…)
+	# MUST be executed as single command, or env vars will not propagate to test execution
+	$(Q) $(GO) test $(GO_DIRS_TO_TEST) # TODO: better way to just run integration tests?
+
+.PHONY: test-int-no-cleanup-cover
+test-int-no-cleanup-cover: $(GCOV2LCOV) ; $(info $(M) running controller integration test…)
+	# MUST be executed as single command, or env vars will not propagate to test execution
+	$(Q) $(GO) test -race -covermode atomic -coverprofile=cover.out -coverpkg=./... $(GO_DIRS_TO_TEST)
+	$(Q) $(GCOV2LCOV) -infile cover.out -outfile coverage.lcov
+
+.PHONY: test-int
+test-int: kind-create deploy test-int-no-cleanup kind-delete cleanup-test-azure-resources
+
+.PHONY: test-int-cover
+test-int-cover: kind-create deploy test-int-no-cleanup-cover kind-delete cleanup-test-azure-resources
+
+# Cleanup resource groups created by tests -- this isn't strictly required as the tests
+# clean up after themselves, but doing it here anyway just to be doubly sure we don't leak
+# resources in cases where the test pass is terminated, panics, etc
+# This finds all resource groups which match the specified pattern (k8sinfratest) and are older than a day
+# (86400 seconds). This is a bit horrible but it works...
+.PHONY: cleanup-test-azure-resources
+cleanup-test-azure-resources:
+	$(Q) rgs=`az group list --query "[*].{Name: name, CreatedAt: tags.CreatedAt}" \
+		| jq -r '.[] | select(.Name | test("^k8sinfratest")) | select(.CreatedAt == null or now-(.CreatedAt | fromdate) > 86400) | .Name'`; \
+	for rgname in $${rgs[@]} ; do \
+		echo "$$rgname will be deleted"; \
+		az group delete --name $$rgname --no-wait --yes; \
+	done
 
 ## --------------------------------------
 ## Development
@@ -90,16 +132,7 @@ kind-delete: $(KIND) ## Destroys the "k8sinfra" kind cluster.
 
 .PHONY: kind-create
 kind-create:
-	$(Q) $(KIND) get clusters | grep -E $(KIND_CLUSTER_NAME) > /dev/null;\
-	EXISTS=$$?;\
-	if [ $$EXISTS -eq 0 ]; then \
-		echo "$(KIND_CLUSTER_NAME) already exists"; \
-	else \
-		$(KIND) create cluster --name=$(KIND_CLUSTER_NAME); \
-	fi; \
-
-	# TODO: Need to use this script when we actually start installing stuff from a registry
-	# $(SCRIPTS_DIR)/kind-with-registry.sh
+	export KIND_CLUSTER_NAME=$(KIND_CLUSTER_NAME) && $(SCRIPTS_DIR)/kind-with-registry.sh
 
 # TODO: We may want this later
 #.PHONY: apply-certs-and-secrets
@@ -109,7 +142,7 @@ kind-create:
 .PHONY: run
 run: export ENVIRONMENT = development
 run: kind-create install ## Run a development cluster using kind
-	$(GO) run ./main.go -v 4
+	$(Q) $(GO) run ./main.go -v $(CONTROLLER_DEBUG_LOGLEVEL)
 
 ## --------------------------------------
 ## Deploy
@@ -125,7 +158,9 @@ uninstall: generate $(KUBECTL) $(KUSTOMIZE) ## Uninstall CRDs from a cluster
 
 .PHONY: deploy
 deploy: generate $(KUBECTL) $(KUSTOMIZE) docker-build docker-push ## Deploy controller in the configured Kubernetes cluster in ~/.kube/config
+	# TODO: Consider patching in CONTROLLER_DEBUG_LOGLEVEL?
 	$(KUSTOMIZE) build config/default | sed "s_${CONFIG_REGISTRY}_${REGISTRY}/${IMG}_" | $(KUBECTL) apply -f -
+	$(SCRIPTS_DIR)/deploy_testing_secret.sh
 
 .PHONY: docker-build
 docker-build: ## Build the docker image

hack/generated/config/default/kustomization.yaml

@@ -6,16 +6,16 @@ namespace: k8s-infra-system
 # "wordpress" becomes "alices-wordpress".
 # Note that it should also match with the prefix (text before '-') of the namespace
 # field above.
-# namePrefix: k8s-infra-
+namePrefix: k8s-infra-
 
 # Labels to add to all resources and selectors.
 #commonLabels:
 #  someName: someValue
 
 bases:
 - ../crd
-# - ../rbac
-# - ../manager
+- ../rbac
+- ../manager
 # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in crd/kustomization.yaml
 # - ../webhook
 # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required.

hack/generated/config/manager/kustomization.yaml

@@ -0,0 +1,7 @@
+resources:
+- manager.yaml
+
+patchesStrategicMerge:
+  - manager_auth_proxy_patch.yaml
+  - manager_image_patch.yaml
+  - manager_pull_policy.yaml

hack/generated/config/manager/manager.yaml

@@ -0,0 +1,39 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  labels:
+    control-plane: controller-manager
+  name: system
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: controller-manager
+  namespace: system
+  labels:
+    control-plane: controller-manager
+spec:
+  selector:
+    matchLabels:
+      control-plane: controller-manager
+  replicas: 1
+  template:
+    metadata:
+      labels:
+        control-plane: controller-manager
+    spec:
+      containers:
+      - # command:
+        # - /manager
+        args:
+        - --enable-leader-election
+        image: controller:latest
+        name: manager
+        resources:
+          limits:
+            cpu: 500m
+            memory: 512Mi
+          requests:
+            cpu: 200m
+            memory: 256Mi
+      terminationGracePeriodSeconds: 10

hack/generated/config/manager/manager_auth_proxy_patch.yaml

@@ -0,0 +1,25 @@
+# This patch inject a sidecar container which is a HTTP proxy for the controller manager,
+# it performs RBAC authorization against the Kubernetes API using SubjectAccessReviews.
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: controller-manager
+  namespace: system
+spec:
+  template:
+    spec:
+      containers:
+      - name: kube-rbac-proxy
+        image: gcr.io/kubebuilder/kube-rbac-proxy:v0.4.1
+        args:
+        - "--secure-listen-address=0.0.0.0:8443"
+        - "--upstream=http://127.0.0.1:8080/"
+        - "--logtostderr=true"
+        - "--v=10"
+        ports:
+        - containerPort: 8443
+          name: https
+      - name: manager
+        args:
+        - "--metrics-addr=127.0.0.1:8080"
+        - "--enable-leader-election"

hack/generated/config/manager/manager_image_patch.yaml

@@ -0,0 +1,33 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: controller-manager
+  namespace: system
+spec:
+  template:
+    spec:
+      containers:
+        # Change the value of image field below to your controller image URL
+        - image: kind-registry:5000/fake/k8s-infra-controller:latest
+          name: manager
+          env:
+            - name: AZURE_CLIENT_ID
+              valueFrom:
+                secretKeyRef:
+                  name: k8sinfra-controller-settings
+                  key: AZURE_CLIENT_ID
+            - name: AZURE_CLIENT_SECRET
+              valueFrom:
+                secretKeyRef:
+                  name: k8sinfra-controller-settings
+                  key: AZURE_CLIENT_SECRET
+            - name: AZURE_TENANT_ID
+              valueFrom:
+                secretKeyRef:
+                  name: k8sinfra-controller-settings
+                  key: AZURE_TENANT_ID
+            - name: AZURE_SUBSCRIPTION_ID
+              valueFrom:
+                secretKeyRef:
+                  name: k8sinfra-controller-settings
+                  key: AZURE_SUBSCRIPTION_ID