Merge pull request #323 from sebsoto/labelBug

Bug 1930791: [wmco] Stop adding pub-key-hash label to all nodes

controllers/secret/secret_controller.go

@@ -59,6 +59,10 @@ func newReconciler(mgr manager.Manager) (reconcile.Reconciler, error) {
 	}
 
 	reconciler := &ReconcileSecret{client: client, scheme: mgr.GetScheme()}
+	if err = reconciler.removeInvalidAnnotationsFromLinuxNodes(); err != nil {
+		log.Error(err, "unable to clean up annotations on Linux nodes")
+	}
+
 	return reconciler, nil
 }
 
@@ -180,7 +184,7 @@ func (r *ReconcileSecret) Reconcile(request reconcile.Request) (reconcile.Result
 			return reconcile.Result{}, errors.Wrap(err, "error creating signer from private key")
 		}
 		nodes := &core.NodeList{}
-		err = r.client.List(context.TODO(), nodes, client.HasLabels{nodeconfig.WindowsOSLabel})
+		err = r.client.List(context.TODO(), nodes, client.MatchingLabels{core.LabelOSStable: "windows"})
 		if err != nil {
 			return reconcile.Result{}, errors.Wrapf(err, "error getting node list")
 		}
@@ -213,6 +217,28 @@ func (r *ReconcileSecret) Reconcile(request reconcile.Request) (reconcile.Result
 	}
 }
 
+// removeInvalidAnnotationsFromLinuxNodes corrects annotations applied by previous versions of WMCO.
+func (r *ReconcileSecret) removeInvalidAnnotationsFromLinuxNodes() error {
+	nodes := &core.NodeList{}
+	err := r.client.List(context.TODO(), nodes, client.MatchingLabels{core.LabelOSStable: "linux"})
+	if err != nil {
+		return errors.Wrapf(err, "error getting node list")
+	}
+	// The public key hash was accidentally added to Linux nodes in WMCO 2.0 and must be removed.
+	// The `/` in the annotation key needs to be escaped in order to not be considered a "directory" in the path.
+	escapedPubKeyAnnotation := strings.Replace(nodeconfig.PubKeyHashAnnotation, "/", "~1", -1)
+	patchData := fmt.Sprintf(`[{"op":"remove","path":"/metadata/annotations/%s"}]`, escapedPubKeyAnnotation)
+	for _, node := range nodes.Items {
+		if _, present := node.Annotations[nodeconfig.PubKeyHashAnnotation]; present == true {
+			err = r.client.Patch(context.TODO(), &node, client.RawPatch(kubeTypes.JSONPatchType, []byte(patchData)))
+			if err != nil {
+				return errors.Wrapf(err, "error removing public key annotation from node %s", node.GetName())
+			}
+		}
+	}
+	return nil
+}
+
 // userDataMapper is a simple implementation of the Mapper interface allowing for the mapping from the userData secret
 // to the private key secret
 type userDataMapper struct {

test/e2e/validation_test.go

@@ -1,6 +1,7 @@
 package e2e
 
 import (
+	"context"
 	"fmt"
 	"os/exec"
 	"strings"
@@ -10,6 +11,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	core "k8s.io/api/core/v1"
+	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	nc "github.com/openshift/windows-machine-config-operator/pkg/nodeconfig"
 )
@@ -57,6 +59,17 @@ func testNodeMetadata(t *testing.T) {
 			})
 		})
 	}
+	t.Run("Windows node metadata not applied to Linux nodes", func(t *testing.T) {
+		nodes, err := tc.client.K8s.CoreV1().Nodes().List(context.TODO(), meta.ListOptions{
+			LabelSelector: core.LabelOSStable + "=linux"})
+		require.NoError(t, err, "error listing Linux nodes")
+		for _, node := range nodes.Items {
+			assert.NotContainsf(t, node.Annotations, nc.VersionAnnotation,
+				"version annotation applied to Linux node %s", node.GetName())
+			assert.NotContainsf(t, node.Annotations, nc.PubKeyHashAnnotation,
+				"public key annotation applied to Linux node %s", node.GetName())
+		}
+	})
 }
 
 // getInstanceID gets the instanceID of VM for a given cloud provider ID