[wmco] Fix pubkey hash annotation logic

This commit fixes an issue where the public key hash annotation was being
added to not all nodes, not just Windows nodes. This commit also adds
logic to remove the annotation from Linux nodes when the operator is
installed.

As part of this fix, Windows node selection in the secret controller
now uses the label "kubernetes.io/os=windows". There is a mix of this
label and "node.openshift.io/os_id=Windows" in the node selection logic
throughout WMCO, and we should move towards just using one. Future work
should be done to only use the "kubernetes.io/os" label for node
selection throughout WMCO.

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