MGMT-19496: Always reconcile ignition token secret (#157)

The secret used as the bearer token for the ignition
server should always be updated or created regardless
of the status of the AgentMachine. This is to prevent
scenarios where the secret is deleted but not recreated
when an AgentMachine is ready.

Co-authored-by: CrystalChun <[email protected]>

Author: openshift-cherrypick-robot

controllers/agentmachine_controller.go

@@ -132,6 +132,11 @@ func (r *AgentMachineReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		return ctrl.Result{}, nil
 	}
 
+	machineConfigPool, ignitionTokenSecretRef, ignitionEndpointHTTPHeaders, err := r.processBootstrapDataSecret(ctx, log, machine, agentMachine.Status.Ready)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+
 	// If the AgentMachine is ready, we have nothing to do
 	if agentMachine.Status.Ready {
 		return ctrl.Result{}, nil
@@ -148,11 +153,6 @@ func (r *AgentMachineReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		return ctrl.Result{}, r.updateStatus(ctx, log, agentMachine, err)
 	}
 
-	machineConfigPool, ignitionTokenSecretRef, ignitionEndpointHTTPHeaders, err := r.processBootstrapDataSecret(ctx, log, machine)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
-
 	// If the AgentMachine doesn't have an agent, find one and set the agentRef
 	if agentMachine.Status.AgentRef == nil {
 		var foundAgent *aiv1beta1.Agent
@@ -439,7 +439,7 @@ func (r *AgentMachineReconciler) updateFoundAgent(ctx context.Context, log logru
 }
 
 func (r *AgentMachineReconciler) processBootstrapDataSecret(ctx context.Context, log logrus.FieldLogger,
-	machine *clusterv1.Machine) (string, *aiv1beta1.IgnitionEndpointTokenReference, map[string]string, error) {
+	machine *clusterv1.Machine, agentMachineReady bool) (string, *aiv1beta1.IgnitionEndpointTokenReference, map[string]string, error) {
 
 	machineConfigPool := ""
 	var ignitionTokenSecretRef *aiv1beta1.IgnitionEndpointTokenReference
@@ -505,6 +505,9 @@ func (r *AgentMachineReconciler) processBootstrapDataSecret(ctx context.Context,
 	ignitionTokenSecretRef = &aiv1beta1.IgnitionEndpointTokenReference{Namespace: machine.Namespace, Name: ignitionTokenSecretName}
 	// TODO: Use a dedicated secret per host and Delete the secret upon cleanup,
 	err := r.Client.Create(ctx, ignitionTokenSecret)
+	if err == nil && agentMachineReady {
+		log.Warnf("ignition token secret %s should not be deleted", ignitionTokenSecret)
+	}
 	if apierrors.IsAlreadyExists(err) {
 		log.Infof("ignitionTokenSecret %s already exists, updating secret content",
 			fmt.Sprintf("agent-%s", *machine.Spec.Bootstrap.DataSecretName))

controllers/agentmachine_controller_test.go

@@ -667,6 +667,109 @@ var _ = Describe("agentmachine reconcile", func() {
 			Expect(agent.Spec.ClusterDeploymentName.Name).To(BeEquivalentTo("cluster-deployment-agentMachine-1"))
 		})
 	})
+	Context("reconciling ignition token secret", func() {
+		var (
+			agent        *aiv1beta1.Agent
+			agentMachine *capiproviderv1.AgentMachine
+			machine      *clusterv1.Machine
+		)
+
+		BeforeEach(func() {
+			agent = newAgent("agent-1", testNamespace, aiv1beta1.AgentSpec{Approved: false})
+			agent.Status.Conditions = append(agent.Status.Conditions, v1.Condition{Type: aiv1beta1.BoundCondition, Status: "True"})
+			agent.Status.Conditions = append(agent.Status.Conditions, v1.Condition{Type: aiv1beta1.ValidatedCondition, Status: "True"})
+			agent.Spec.ClusterDeploymentName = &aiv1beta1.ClusterReference{Name: "cluster-deployment-agentMachine-1", Namespace: testNamespace}
+			agentMachine, machine = newAgentMachine("agentMachine-1", testNamespace, capiproviderv1.AgentMachineSpec{}, ctx, c)
+
+			Expect(c.Create(ctx, agent)).To(BeNil())
+			Expect(c.Create(ctx, agentMachine)).To(BeNil())
+		})
+		AfterEach(func() {
+			mockCtrl.Finish()
+		})
+		It("creates the ignition token secret upon first reconcile of an AgentMachine", func() {
+			result, err := amr.Reconcile(ctx, newAgentMachineRequest(agentMachine))
+			Expect(err).To(BeNil())
+			Expect(result).To(Equal(ctrl.Result{}))
+
+			ignitionTokenSecretName := machine.Spec.Bootstrap.DataSecretName
+			ignitionTokenSecret := &corev1.Secret{}
+			Expect(c.Get(ctx, types.NamespacedName{Name: *ignitionTokenSecretName, Namespace: testNamespace}, ignitionTokenSecret)).To(Succeed())
+			ignitionConfig := &ignitionapi.Config{}
+			Expect(json.Unmarshal(ignitionTokenSecret.Data["value"], ignitionConfig)).To(Succeed())
+			expectedPrefix := "Bearer "
+			ignitionToken := (*ignitionConfig.Ignition.Config.Merge[0].HTTPHeaders[0].Value)[len(expectedPrefix):]
+
+			agentSecret := &corev1.Secret{}
+			Expect(c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("agent-%s", *ignitionTokenSecretName), Namespace: testNamespace}, agentSecret)).To(Succeed())
+			Expect(agentSecret.Data).NotTo(BeEmpty())
+			Expect(agentSecret.Data).To(HaveKey("ignition-token"))
+			Expect(string(agentSecret.Data["ignition-token"])).To(Equal(ignitionToken))
+		})
+		It("updates the ignition token secret if the token changed", func() {
+			result, err := amr.Reconcile(ctx, newAgentMachineRequest(agentMachine))
+			Expect(err).To(BeNil())
+			Expect(result).To(Equal(ctrl.Result{}))
+
+			ignitionTokenSecretName := machine.Spec.Bootstrap.DataSecretName
+			ignitionTokenSecret := &corev1.Secret{}
+			Expect(c.Get(ctx, types.NamespacedName{Name: *ignitionTokenSecretName, Namespace: testNamespace}, ignitionTokenSecret)).To(Succeed())
+			ignitionConfig := &ignitionapi.Config{}
+			Expect(json.Unmarshal(ignitionTokenSecret.Data["value"], ignitionConfig)).To(Succeed())
+			expectedPrefix := "Bearer "
+			ignitionToken := (*ignitionConfig.Ignition.Config.Merge[0].HTTPHeaders[0].Value)[len(expectedPrefix):]
+
+			agentSecret := &corev1.Secret{}
+			Expect(c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("agent-%s", *ignitionTokenSecretName), Namespace: testNamespace}, agentSecret)).To(Succeed())
+			Expect(agentSecret.Data).NotTo(BeEmpty())
+			Expect(agentSecret.Data).To(HaveKey("ignition-token"))
+			Expect(string(agentSecret.Data["ignition-token"])).To(Equal(ignitionToken))
+
+			ignitionConfig.Ignition.Config.Merge[0].HTTPHeaders[0].Value = swag.String("Bearer new-token")
+			updatedIgnitionConfig, err := json.Marshal(ignitionConfig)
+			Expect(err).To(BeNil())
+			ignitionTokenSecret.Data["value"] = updatedIgnitionConfig
+			Expect(c.Update(ctx, ignitionTokenSecret)).To(Succeed())
+
+			result, err = amr.Reconcile(ctx, newAgentMachineRequest(agentMachine))
+			Expect(err).To(BeNil())
+			Expect(result).To(Equal(ctrl.Result{}))
+
+			Expect(c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("agent-%s", *ignitionTokenSecretName), Namespace: testNamespace}, agentSecret)).To(Succeed())
+			Expect(agentSecret.Data).NotTo(BeEmpty())
+			Expect(agentSecret.Data).To(HaveKey("ignition-token"))
+			Expect(string(agentSecret.Data["ignition-token"])).To(Equal("new-token"))
+
+		})
+		It("creates the ignition token secret even when the AgentMachine is already ready", func() {
+			agentMachine.Status.AgentRef = &capiproviderv1.AgentReference{Namespace: agent.Namespace, Name: agent.Name}
+			agentMachine.Status.Ready = true
+			Expect(c.Update(ctx, agentMachine)).To(Succeed())
+
+			ignitionTokenSecretName := machine.Spec.Bootstrap.DataSecretName
+			agentSecret := &corev1.Secret{}
+			err := c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("agent-%s", *ignitionTokenSecretName), Namespace: testNamespace}, agentSecret)
+			Expect(err).To(HaveOccurred())
+			Expect(apierrors.IsNotFound(err)).To(BeTrue())
+
+			result, err := amr.Reconcile(ctx, newAgentMachineRequest(agentMachine))
+			Expect(err).To(BeNil())
+			Expect(result).To(Equal(ctrl.Result{}))
+
+			ignitionTokenSecret := &corev1.Secret{}
+			Expect(c.Get(ctx, types.NamespacedName{Name: *ignitionTokenSecretName, Namespace: testNamespace}, ignitionTokenSecret)).To(Succeed())
+			ignitionConfig := &ignitionapi.Config{}
+			Expect(json.Unmarshal(ignitionTokenSecret.Data["value"], ignitionConfig)).To(Succeed())
+			expectedPrefix := "Bearer "
+			ignitionToken := (*ignitionConfig.Ignition.Config.Merge[0].HTTPHeaders[0].Value)[len(expectedPrefix):]
+
+			Expect(c.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("agent-%s", *ignitionTokenSecretName), Namespace: testNamespace}, agentSecret)).To(Succeed())
+			Expect(agentSecret.Data).NotTo(BeEmpty())
+			Expect(agentSecret.Data).To(HaveKey("ignition-token"))
+			Expect(string(agentSecret.Data["ignition-token"])).To(Equal(ignitionToken))
+
+		})
+	})
 })
 
 var _ = Describe("mapMachineToAgentMachine", func() {