Skip to content

Commit

Permalink
Improve validation for AzureSQLUser and AzureSQLManagerUser (#1348)
Browse files Browse the repository at this point in the history
* Kubebuilder annotation for minlength should use = not :

* Add additional validations on MySQL users

* Add validations to more Azure SQL types

* Update some dependencies

  - We're primarily interested in updating controller-runtime
    as the newer version supports webhooks in envtest better.

* Don't allow DB name "master" to be used for Azure SQL users

* Fixes for CI
  • Loading branch information
matthchr authored Jan 12, 2021
1 parent 892b362 commit 322e7e8
Show file tree
Hide file tree
Showing 61 changed files with 509 additions and 149 deletions.
51 changes: 16 additions & 35 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ generate-test-certs:
# Run Controller tests against the configured cluster
.PHONY: test-integration-controllers
test-integration-controllers: generate fmt vet manifests
TEST_RESOURCE_PREFIX=$(TEST_RESOURCE_PREFIX) TEST_USE_EXISTING_CLUSTER=true REQUEUE_AFTER=20 \
TEST_RESOURCE_PREFIX=$(TEST_RESOURCE_PREFIX) TEST_USE_EXISTING_CLUSTER=false REQUEUE_AFTER=20 \
go test -v -tags "$(BUILD_TAGS)" -coverprofile=reports/integration-controllers-coverage-output.txt -coverpkg=./... -covermode count -parallel 4 -timeout 45m \
./controllers/...
#2>&1 | tee reports/integration-controllers-output.txt
Expand Down Expand Up @@ -242,11 +242,11 @@ generate-template:
# TODO: These kind-delete / kind-create targets were stolen from k8s-infra and
# TODO: should be merged back together when the projects more closely align
.PHONY: kind-delete
kind-delete: install-kind
kind-delete: install-test-dependencies
kind delete cluster --name=$(KIND_CLUSTER_NAME) || true

.PHONY: kind-create
kind-create: install-kind
kind-create: install-test-dependencies
kind get clusters | grep -E $(KIND_CLUSTER_NAME) > /dev/null;\
EXISTS=$$?;\
if [ $$EXISTS -eq 0 ]; then \
Expand All @@ -256,12 +256,12 @@ kind-create: install-kind
fi; \

.PHONY: set-kindcluster
set-kindcluster: install-kind kind-create
ifeq (${shell kind get kubeconfig-path --name="kind"},${KUBECONFIG})
set-kindcluster: kind-create
ifeq (${shell kind get kubeconfig-path --name=$(KIND_CLUSTER_NAME)},${KUBECONFIG})
@echo "kubeconfig-path points to kind path"
else
@echo "please run below command in your shell and then re-run make set-kindcluster"
@echo "\e[31mexport KUBECONFIG=$(shell kind get kubeconfig-path --name="kind")\e[0m"
@echo "\e[31mexport KUBECONFIG=$(shell kind get kubeconfig-path --name="$(KIND_CLUSTER_NAME)")\e[0m"
@exit 111
endif
@echo "getting value of KUBECONFIG"
Expand All @@ -282,7 +282,7 @@ endif
#create image and load it into cluster
make install
IMG="docker.io/controllertest:1" make docker-build
kind load docker-image docker.io/controllertest:1 --loglevel "trace"
kind load docker-image docker.io/controllertest:1 --loglevel "trace" --name=$(KIND_CLUSTER_NAME)

kubectl get namespaces
kubectl get pods --namespace cert-manager
Expand All @@ -292,48 +292,25 @@ endif
make deploy
sed -i'' -e 's@image: .*@image: '"IMAGE_URL"'@' ./config/default/manager_image_patch.yaml

.PHONY: install-kind
install-kind:
ifeq (,$(shell which kind))
@echo "installing kind"
GO111MODULE="on" go get sigs.k8s.io/kind@v0.8.1
else
@echo "kind has been installed"
endif

.PHONY: install-kubebuilder
install-kubebuilder:
ifeq (,$(shell which kubebuilder))
@echo "installing kubebuilder"
# download kubebuilder and extract it to tmp
curl -sL https://go.kubebuilder.io/dl/2.0.0/$(shell go env GOOS)/$(shell go env GOARCH) | tar -xz -C $(TMPDIR)
curl -sL https://go.kubebuilder.io/dl/2.3.1/$(shell go env GOOS)/$(shell go env GOARCH) | tar -xz -C $(TMPDIR)
# move to a long-term location and put it on your path
# (you'll need to set the KUBEBUILDER_ASSETS env var if you put it somewhere else)
mv $(TMPDIR)/kubebuilder_2.0.0_$(shell go env GOOS)_$(shell go env GOARCH) /usr/local/kubebuilder
export PATH=$$PATH:/usr/local/kubebuilder/bin
mv $(TMPDIR)/kubebuilder_2.3.1_$(shell go env GOOS)_$(shell go env GOARCH) $(shell go env GOPATH)/kubebuilder
export PATH=$$PATH:$(shell go env GOPATH)/kubebuilder/bin
else
@echo "kubebuilder has been installed"
endif

.PHONY: install-kustomize
install-kustomize:
ifeq (,$(shell which kustomize))
@echo "installing kustomize"
mkdir -p /usr/local/kubebuilder/bin
# download kustomize
curl -o /usr/local/kubebuilder/bin/kustomize -sL "https://go.kubebuilder.io/kustomize/$(shell go env GOOS)/$(shell go env GOARCH)"
# set permission
chmod a+x /usr/local/kubebuilder/bin/kustomize
$(shell which kustomize)
else
@echo "kustomize has been installed"
endif

.PHONY: install-cert-manager
install-cert-manager:
kubectl create namespace cert-manager
kubectl label namespace cert-manager cert-manager.io/disable-validation=true
kubectl apply --validate=false -f https://github.com/jetstack/cert-manager/releases/download/v0.12.0/cert-manager.yaml
kubectl apply -f https://github.com/jetstack/cert-manager/releases/download/v1.1.0/cert-manager.yaml

.PHONY: install-aad-pod-identity
install-aad-pod-identity:
Expand All @@ -344,7 +321,11 @@ install-test-dependencies:
go get github.com/jstemmer/go-junit-report \
&& go get github.com/axw/gocov/gocov \
&& go get github.com/AlekSi/gocov-xml \
&& go get github.com/wadey/gocovmerge
&& go get github.com/wadey/gocovmerge \
&& go get k8s.io/code-generator/cmd/conversion-gen@v0.18.2 \
&& go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.4.0 \
&& go get sigs.k8s.io/kind@v0.9.0 \
&& go get sigs.k8s.io/kustomize/kustomize/v3@v3.8.6

# Operator-sdk release version
RELEASE_VERSION ?= v1.0.1
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/apimgmt_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
type APIMgmtSpec struct {
Location string `json:"location"`
// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourceGroup"`
APIService string `json:"apiService"`
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/appinsights_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type AppInsightsSpec struct {
Location string `json:"location"`
ApplicationType string `json:"applicationType"` // Possible values include 'web' or 'other'
// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourceGroup"`
KeyVaultToStoreSecrets string `json:"keyVaultToStoreSecrets,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/azureloadbalancer_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type AzureLoadBalancerSpec struct {
// Important: Run "make" to regenerate code after modifying this file
Location string `json:"location"`
// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourceGroup"`
PublicIPAddressName string `json:"publicIPAddressName"`
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/azurenetworkinterface_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type AzureNetworkInterfaceSpec struct {
// Important: Run "make" to regenerate code after modifying this file
Location string `json:"location"`
// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourceGroup"`
VNetName string `json:"vnetName"`
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/azurepublicipaddress_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type AzurePublicIPAddressSpec struct {

Location string `json:"location"`
// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourceGroup"`
PublicIPAllocationMethod string `json:"publicIPAllocationMethod"`
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/azuresqlaction_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type AzureSqlActionSpec struct {
// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
// Important: Run "make" to regenerate code after modifying this file
// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourceGroup"`
ActionName string `json:"actionName"`
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/azuresqldatabase_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type AzureSqlDatabaseSpec struct {
// Important: Run "make" to regenerate code after modifying this file
Location string `json:"location"`
// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourcegroup"`
Server string `json:"server"`
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/azuresqlfailovergroup_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ type AzureSqlFailoverGroupSpec struct {
// Important: Run "make" to regenerate code after modifying this file
Location string `json:"location"`
// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourcegroup"`
Server string `json:"server"`
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/azuresqlfirewallrule_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type AzureSqlFirewallRuleSpec struct {
// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
// Important: Run "make" to regenerate code after modifying this file
// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourcegroup"`
Server string `json:"server"`
Expand Down
24 changes: 17 additions & 7 deletions api/v1alpha1/azuresqlmanageduser_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,27 @@ import (
type AzureSQLManagedUserSpec struct {
// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
// Important: Run "make" to regenerate code after modifying this file

// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
Server string `json:"server"`

// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
DbName string `json:"dbName"`

// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourceGroup"`

// +kubebuilder:validation:Required
ResourceGroup string `json:"resourceGroup"`
Roles []string `json:"roles"`
ManagedIdentityName string `json:"managedIdentityName,omitempty"`
ManagedIdentityClientId string `json:"managedIdentityClientId"`
KeyVaultSecretPrefix string `json:"keyVaultSecretPrefix,omitempty"`
KeyVaultToStoreSecrets string `json:"keyVaultToStoreSecrets,omitempty"`
Roles []string `json:"roles"`

ManagedIdentityName string `json:"managedIdentityName,omitempty"`
ManagedIdentityClientId string `json:"managedIdentityClientId"`
KeyVaultSecretPrefix string `json:"keyVaultSecretPrefix,omitempty"`
KeyVaultToStoreSecrets string `json:"keyVaultToStoreSecrets,omitempty"`
}

// +kubebuilder:object:root=true
Expand Down
46 changes: 46 additions & 0 deletions api/v1alpha1/azuresqlmanageduser_webhook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

package v1alpha1

import (
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)

// log is for logging in this package.
var azuresqlmanageduserlog = logf.Log.WithName("azuresqlmanageduser-resource")

func (r *AzureSQLManagedUser) SetupWebhookWithManager(mgr ctrl.Manager) error {
return ctrl.NewWebhookManagedBy(mgr).
For(r).
Complete()
}

// +kubebuilder:webhook:verbs=create;update,path=/validate-azure-microsoft-com-v1alpha1-azuresqlmanageduser,mutating=false,failurePolicy=fail,groups=azure.microsoft.com,resources=azuresqlmanagedusers,versions=v1alpha1,name=vazuresqlmanageduser.kb.io

var _ webhook.Validator = &AzureSQLManagedUser{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (r *AzureSQLManagedUser) ValidateCreate() error {
azuresqlmanageduserlog.Info("validate create", "name", r.Name)

return ValidateAzureSQLDBName(r.Spec.DbName)
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (r *AzureSQLManagedUser) ValidateUpdate(old runtime.Object) error {
azuresqlmanageduserlog.Info("validate update", "name", r.Name)

return ValidateAzureSQLDBName(r.Spec.DbName)
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (r *AzureSQLManagedUser) ValidateDelete() error {
azuresqlmanageduserlog.Info("validate delete", "name", r.Name)

// TODO(user): fill in your validation logic upon object deletion.
return nil
}
2 changes: 1 addition & 1 deletion api/v1alpha1/azuresqlserver_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type AzureSqlServerSpec struct {
// Important: Run "make" to regenerate code after modifying this file
Location string `json:"location"`
// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourcegroup"`
KeyVaultToStoreSecrets string `json:"keyVaultToStoreSecrets,omitempty"`
Expand Down
16 changes: 13 additions & 3 deletions api/v1alpha1/azuresqluser_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,23 @@ import (
type AzureSQLUserSpec struct {
// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
// Important: Run "make" to regenerate code after modifying this file

// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
Server string `json:"server"`

// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
DbName string `json:"dbName"`

// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourceGroup"`

// +kubebuilder:validation:Required
ResourceGroup string `json:"resourceGroup"`
Roles []string `json:"roles"`
Roles []string `json:"roles"`

// optional
AdminSecret string `json:"adminSecret,omitempty"`
AdminSecretKeyVault string `json:"adminSecretKeyVault,omitempty"`
Expand Down
55 changes: 55 additions & 0 deletions api/v1alpha1/azuresqluser_webhook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

package v1alpha1

import (
"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)

// log is for logging in this package.
var azuresqluserlog = logf.Log.WithName("azuresqluser-resource")

func (r *AzureSQLUser) SetupWebhookWithManager(mgr ctrl.Manager) error {
return ctrl.NewWebhookManagedBy(mgr).
For(r).
Complete()
}

// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!

// +kubebuilder:webhook:verbs=create;update,path=/validate-azure-microsoft-com-v1alpha1-azuresqluser,mutating=false,failurePolicy=fail,groups=azure.microsoft.com,resources=azuresqlusers,versions=v1alpha1,name=vazuresqluser.kb.io

func ValidateAzureSQLDBName(name string) error {
if name == "master" {
return errors.Errorf("'master' is a reserved database name and cannot be used")
}

return nil
}

var _ webhook.Validator = &AzureSQLUser{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (r *AzureSQLUser) ValidateCreate() error {
azuresqluserlog.Info("validate create", "name", r.Name)

return ValidateAzureSQLDBName(r.Spec.DbName)
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (r *AzureSQLUser) ValidateUpdate(old runtime.Object) error {
azuresqluserlog.Info("validate update", "name", r.Name)

return ValidateAzureSQLDBName(r.Spec.DbName)
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (r *AzureSQLUser) ValidateDelete() error {
azuresqluserlog.Info("validate delete", "name", r.Name)
return nil
}
6 changes: 4 additions & 2 deletions api/v1alpha1/azuresqlvnetrule_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ type AzureSQLVNetRuleSpec struct {
// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
// Important: Run "make" to regenerate code after modifying this file
// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourceGroup"`
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourceGroup"`
Server string `json:"server"`
VNetResourceGroup string `json:"vNetResourceGroup"`
VNetName string `json:"vNetName"`
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/azurevirtualmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type AzureVirtualMachineSpec struct {
// Important: Run "make" to regenerate code after modifying this file
Location string `json:"location"`
// +kubebuilder:validation:Pattern=^[-\w\._\(\)]+$
// +kubebuilder:validation:MinLength:1
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Required
ResourceGroup string `json:"resourceGroup"`
VMSize string `json:"vmSize"`
Expand Down
Loading

0 comments on commit 322e7e8

Please sign in to comment.