Ensure wordboundary present on regexp

Author: nicholasjackson

.gitignore

@@ -1,3 +1,4 @@
 node_modules
 docs/.docusaurus
-docs/build
+docs/build
+functional_tests/tests.log

functional_tests/tests.log

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"regexp"
+	"strings"
 
 	"github.com/hashicorp/go-hclog"
 	"github.com/nicholasjackson/consul-release-controller/plugins/interfaces"
@@ -42,7 +43,10 @@ func (a *deploymentAdmission) Handle(ctx context.Context, req admission.Request)
 	a.log.Debug("Handle deployment admission", "deployment", deployment.Name, "namespaces", deployment.Namespace)
 
 	// was the deployment modified by the release controller, if so, ignore
-	if deployment.Labels != nil && deployment.Labels["consul-release-controller-version"] != "" && deployment.Labels["consul-release-controller-version"] == deployment.ResourceVersion {
+	if deployment.Labels != nil &&
+		deployment.Labels[interfaces.RuntimeDeploymentVersionLabel] != "" &&
+		deployment.Labels[interfaces.RuntimeDeploymentVersionLabel] == deployment.ResourceVersion {
+
 		a.log.Debug("Ignore deployment, resource was modified by the controller", "name", deployment.Name, "namespace", deployment.Namespace, "labels", deployment.Labels)
 
 		return admission.Allowed("resource modified by controller")
@@ -61,12 +65,20 @@ func (a *deploymentAdmission) Handle(ctx context.Context, req admission.Request)
 
 		// PluginConfig.Deployment can reference deployments using regular expressions
 		// check if this matches
+
+		//first check to see if the regex terminates in $ (word boundary), if not add it
+		if !strings.HasSuffix(conf.Deployment, "$") {
+			conf.Deployment = conf.Deployment + "$"
+		}
+
 		re, err := regexp.Compile(conf.Deployment)
 		if err != nil {
 			a.log.Error("Invalid regular expression for deployment in release config", "release", rel.Name, "error", err)
 			continue
 		}
 
+		a.log.Debug("Checking release", "name", deployment.Name, "namespace", deployment.Namespace, "regex", conf.Deployment)
+
 		if re.MatchString(deployment.Name) && conf.Namespace == deployment.Namespace {
 			// found a release for this deployment, check the state
 			sm, err := a.provider.GetStateMachine(rel)
@@ -75,7 +87,7 @@ func (a *deploymentAdmission) Handle(ctx context.Context, req admission.Request)
 				return admission.Errored(500, err)
 			}
 
-			a.log.Debug("Found existing release", "name", deployment.Name, "namespace", deployment.Namespace, "state", sm.CurrentState())
+			a.log.Debug("Found existing release for", "name", deployment.Name, "namespace", deployment.Namespace, "state", sm.CurrentState())
 
 			if sm.CurrentState() == interfaces.StateIdle || sm.CurrentState() == interfaces.StateFail {
 				// kick off a new deployment

kubernetes/controller/validatingwebhook.go

@@ -101,6 +101,18 @@ func TestCallsDeployForNewDeploymentWhenIdle(t *testing.T) {
 	mm.StateMachineMock.AssertCalled(t, "Deploy")
 }
 
+func TestAddsRegExpWordBoundaryAndFailsMatchWhenNotPresent(t *testing.T) {
+	ar := createAdmissionRequest(false)
+
+	// a regexp without a word boundary would match, check we add
+	// the word boundary when not present
+	d, mm := setupAdmission(t, "test-", "default")
+
+	resp := d.Handle(context.TODO(), ar)
+	require.True(t, resp.Allowed)
+	mm.StateMachineMock.AssertNotCalled(t, "Deploy")
+}
+
 func TestCallsDeployForNewDeploymentWhenIdleAndUsingRegularExpressions(t *testing.T) {
 	ar := createAdmissionRequest(false)
 	d, mm := setupAdmission(t, "test-(.*)", "default")

kubernetes/controller/validatingwebhook_test.go

@@ -11,6 +11,7 @@ const (
 	RuntimeDeploymentNoAction      RuntimeDeploymentStatus = "runtime_deployment_no_action"
 	RuntimeDeploymentNotFound      RuntimeDeploymentStatus = "runtime_deployment_not_found"
 	RuntimeDeploymentInternalError RuntimeDeploymentStatus = "runtime_deployment_internal_error"
+	RuntimeDeploymentVersionLabel                          = "consul-release-controller-version"
 )
 
 // RuntimeBaseConfig is the base configuration that all runtime plugins must implement

plugins/interfaces/runtime.go

@@ -108,6 +108,13 @@ func (p *Plugin) InitPrimary(ctx context.Context) (interfaces.RuntimeDeploymentS
 	primaryDeployment.Name = primaryName
 	primaryDeployment.ResourceVersion = ""
 
+	// add labels to ensure the deployment is not picked up by the validating webhook
+	if primaryDeployment.Labels == nil {
+		primaryDeployment.Labels = map[string]string{}
+	}
+
+	primaryDeployment.Labels[interfaces.RuntimeDeploymentVersionLabel] = "1"
+
 	// save the new primary
 	err = p.kubeClient.UpsertDeployment(ctx, primaryDeployment)
 	if err != nil {
@@ -163,6 +170,13 @@ func (p *Plugin) PromoteCandidate(ctx context.Context) (interfaces.RuntimeDeploy
 	primary.Name = primaryName
 	primary.ResourceVersion = ""
 
+	// add labels to ensure the deployment is not picked up by the validating webhook
+	if primary.Labels == nil {
+		primary.Labels = map[string]string{}
+	}
+
+	primary.Labels[interfaces.RuntimeDeploymentVersionLabel] = "1"
+
 	// save the new deployment
 	err = p.kubeClient.UpsertDeployment(ctx, primary)
 	if err != nil {
@@ -255,6 +269,9 @@ func (p *Plugin) RestoreOriginal(ctx context.Context) error {
 	cd.Name = p.config.Deployment
 	cd.ResourceVersion = ""
 
+	// remove the ownership label so that it can be updated as normal
+	delete(cd.Labels, interfaces.RuntimeDeploymentVersionLabel)
+
 	p.log.Debug("Clone primary to create original deployment", "name", p.config.Deployment, "namespace", p.config.Namespace)
 
 	err = p.kubeClient.UpsertDeployment(ctx, cd)

plugins/kubernetes/plugin.go

@@ -17,7 +17,7 @@ import (
 )
 
 var one = int32(1)
-var dep = &appsv1.Deployment{
+var mockDep = &appsv1.Deployment{
 	ObjectMeta: v1.ObjectMeta{
 		Name:      "test-deployment",
 		Namespace: "testnamespace",
@@ -26,7 +26,7 @@ var dep = &appsv1.Deployment{
 	Spec:   appsv1.DeploymentSpec{Replicas: &one},
 }
 
-var cloneDep = &appsv1.Deployment{
+var mockCloneDep = &appsv1.Deployment{
 	ObjectMeta: v1.ObjectMeta{
 		Name:      "test-deployment-primary",
 		Namespace: "testnamespace",
@@ -35,7 +35,7 @@ var cloneDep = &appsv1.Deployment{
 	Spec:   appsv1.DeploymentSpec{Replicas: &one},
 }
 
-func setupPlugin(t *testing.T) (*Plugin, *clients.KubernetesMock) {
+func setupPlugin(t *testing.T) (*Plugin, *clients.KubernetesMock, *appsv1.Deployment, *appsv1.Deployment) {
 
 	retryTimeout = 10 * time.Millisecond
 	retryInterval = 1 * time.Millisecond
@@ -52,11 +52,11 @@ func setupPlugin(t *testing.T) (*Plugin, *clients.KubernetesMock) {
 	p.config.Deployment = "test-deployment"
 	p.config.Namespace = "testnamespace"
 
-	return p, km
+	return p, km, mockDep.DeepCopy(), mockCloneDep.DeepCopy()
 }
 
 func TestInitPrimaryDoesNothingWhenPrimaryExists(t *testing.T) {
-	p, km := setupPlugin(t)
+	p, km, dep, cloneDep := setupPlugin(t)
 
 	testutils.ClearMockCall(&km.Mock, "GetDeployment")
 	km.On("GetDeployment", mock.Anything, "test-deployment-primary", "testnamespace").Return(cloneDep, nil)
@@ -72,7 +72,7 @@ func TestInitPrimaryDoesNothingWhenPrimaryExists(t *testing.T) {
 }
 
 func TestInitPrimaryDoesNothingWhenCandidateDoesNotExist(t *testing.T) {
-	p, km := setupPlugin(t)
+	p, km, _, _ := setupPlugin(t)
 
 	testutils.ClearMockCall(&km.Mock, "GetDeployment")
 	km.On("GetDeployment", mock.Anything, "test-deployment-primary", "testnamespace").Return(nil, fmt.Errorf("Primary not found"))
@@ -84,7 +84,7 @@ func TestInitPrimaryDoesNothingWhenCandidateDoesNotExist(t *testing.T) {
 }
 
 func TestInitPrimaryCreatesPrimaryWhenCandidateExists(t *testing.T) {
-	p, km := setupPlugin(t)
+	p, km, dep, cloneDep := setupPlugin(t)
 
 	testutils.ClearMockCall(&km.Mock, "GetDeployment")
 	km.On("GetDeployment", mock.Anything, "test-deployment-primary", "testnamespace").Once().Return(nil, fmt.Errorf("Primary not found"))
@@ -97,10 +97,14 @@ func TestInitPrimaryCreatesPrimaryWhenCandidateExists(t *testing.T) {
 	require.Equal(t, interfaces.RuntimeDeploymentUpdate, status)
 
 	km.AssertCalled(t, "UpsertDeployment", mock.Anything, mock.Anything)
+
+	// check that the runtimedeploymentversion label is added to ensure the validating webhook ignores this deployment
+	depArg := getUpsertDeployment(km.Mock)
+	require.Equal(t, depArg.Labels[interfaces.RuntimeDeploymentVersionLabel], "1")
 }
 
 func TestPromoteCandidateDoesNothingWhenCandidateNotExists(t *testing.T) {
-	p, km := setupPlugin(t)
+	p, km, _, _ := setupPlugin(t)
 
 	testutils.ClearMockCall(&km.Mock, "GetDeployment")
 	km.On("GetHealthyDeployment", mock.Anything, "test-deployment", "testnamespace").Return(nil, clients.ErrDeploymentNotFound)
@@ -113,7 +117,7 @@ func TestPromoteCandidateDoesNothingWhenCandidateNotExists(t *testing.T) {
 }
 
 func TestPromoteCandidateDeletesExistingPrimaryAndUpserts(t *testing.T) {
-	p, km := setupPlugin(t)
+	p, km, dep, _ := setupPlugin(t)
 
 	testutils.ClearMockCall(&km.Mock, "GetDeployment")
 	km.On("GetHealthyDeployment", mock.Anything, "test-deployment", "testnamespace").Once().Return(dep, nil)
@@ -127,10 +131,14 @@ func TestPromoteCandidateDeletesExistingPrimaryAndUpserts(t *testing.T) {
 
 	km.AssertCalled(t, "DeleteDeployment", mock.Anything, "test-deployment-primary", "testnamespace")
 	km.AssertCalled(t, "UpsertDeployment", mock.Anything, mock.Anything)
+
+	// check that the runtimedeploymentversion label is added to ensure the validating webhook ignores this deployment
+	depArg := getUpsertDeployment(km.Mock)
+	require.Equal(t, depArg.Labels[interfaces.RuntimeDeploymentVersionLabel], "1")
 }
 
 func TestRemoveCandidateDoesNothingWhenCandidateNotFound(t *testing.T) {
-	p, km := setupPlugin(t)
+	p, km, _, _ := setupPlugin(t)
 
 	testutils.ClearMockCall(&km.Mock, "GetDeployment")
 	km.On("GetDeployment", mock.Anything, "test-deployment", "testnamespace").Once().Return(nil, clients.ErrDeploymentNotFound)
@@ -142,7 +150,7 @@ func TestRemoveCandidateDoesNothingWhenCandidateNotFound(t *testing.T) {
 }
 
 func TestRemoveCandidateScalesWhenCandidateFound(t *testing.T) {
-	p, km := setupPlugin(t)
+	p, km, dep, _ := setupPlugin(t)
 
 	testutils.ClearMockCall(&km.Mock, "GetDeployment")
 	km.On("GetDeployment", mock.Anything, "test-deployment", "testnamespace").Once().Return(dep, nil)
@@ -156,7 +164,7 @@ func TestRemoveCandidateScalesWhenCandidateFound(t *testing.T) {
 }
 
 func TestRestoreDoesNothingWhenNoPrimaryFound(t *testing.T) {
-	p, km := setupPlugin(t)
+	p, km, _, _ := setupPlugin(t)
 
 	testutils.ClearMockCall(&km.Mock, "GetDeployment")
 	km.On("GetDeployment", mock.Anything, "test-deployment-primary", "testnamespace").Once().Return(nil, clients.ErrDeploymentNotFound)
@@ -166,7 +174,7 @@ func TestRestoreDoesNothingWhenNoPrimaryFound(t *testing.T) {
 }
 
 func TestRestoreCallsDeleteWhenPrimaryFound(t *testing.T) {
-	p, km := setupPlugin(t)
+	p, km, _, cloneDep := setupPlugin(t)
 
 	testutils.ClearMockCall(&km.Mock, "GetDeployment")
 	km.On("GetDeployment", mock.Anything, "test-deployment-primary", "testnamespace").Once().Return(cloneDep, nil)
@@ -176,10 +184,14 @@ func TestRestoreCallsDeleteWhenPrimaryFound(t *testing.T) {
 
 	err := p.RestoreOriginal(context.Background())
 	require.NoError(t, err)
+
+	// check that the labels are removed
+	depArg := getUpsertDeployment(km.Mock)
+	require.Equal(t, depArg.Labels[interfaces.RuntimeDeploymentVersionLabel], "")
 }
 
 func TestRestoreProceedesWhenExistingCandidateNotFound(t *testing.T) {
-	p, km := setupPlugin(t)
+	p, km, _, cloneDep := setupPlugin(t)
 
 	testutils.ClearMockCall(&km.Mock, "GetDeployment")
 	km.On("GetDeployment", mock.Anything, "test-deployment-primary", "testnamespace").Once().Return(cloneDep, nil)
@@ -192,7 +204,7 @@ func TestRestoreProceedesWhenExistingCandidateNotFound(t *testing.T) {
 }
 
 func TestRemovePrimaryCallsDelete(t *testing.T) {
-	p, km := setupPlugin(t)
+	p, km, _, _ := setupPlugin(t)
 
 	testutils.ClearMockCall(&km.Mock, "GetDeployment")
 	km.On("DeleteDeployment", mock.Anything, "test-deployment-primary", "testnamespace").Once().Return(nil)
@@ -202,7 +214,7 @@ func TestRemovePrimaryCallsDelete(t *testing.T) {
 }
 
 func TestRemovePrimaryReturnsErrorWhenDeleteIsGenericError(t *testing.T) {
-	p, km := setupPlugin(t)
+	p, km, _, _ := setupPlugin(t)
 
 	testutils.ClearMockCall(&km.Mock, "GetDeployment")
 	km.On("DeleteDeployment", mock.Anything, "test-deployment-primary", "testnamespace").Once().Return(fmt.Errorf("test"))
@@ -212,11 +224,23 @@ func TestRemovePrimaryReturnsErrorWhenDeleteIsGenericError(t *testing.T) {
 }
 
 func TestRemovePrimaryReturnsNoErrorWhenDeleteIsNotFoundError(t *testing.T) {
-	p, km := setupPlugin(t)
+	p, km, _, _ := setupPlugin(t)
 
 	testutils.ClearMockCall(&km.Mock, "GetDeployment")
 	km.On("DeleteDeployment", mock.Anything, "test-deployment-primary", "testnamespace").Once().Return(clients.ErrDeploymentNotFound)
 
 	err := p.RemovePrimary(context.Background())
 	require.NoError(t, err)
 }
+
+func getUpsertDeployment(mock mock.Mock) *appsv1.Deployment {
+	for _, c := range mock.Calls {
+		if c.Method == "UpsertDeployment" {
+			if dep, ok := c.Arguments.Get(1).(*appsv1.Deployment); ok {
+				return dep
+			}
+		}
+	}
+
+	return nil
+}