diff --git a/install/0000_80_machine-config-operator_01_machineconfigpool.crd.yaml b/install/0000_80_machine-config-operator_01_machineconfigpool.crd.yaml index 16f24c0879..0822b07198 100644 --- a/install/0000_80_machine-config-operator_01_machineconfigpool.crd.yaml +++ b/install/0000_80_machine-config-operator_01_machineconfigpool.crd.yaml @@ -427,3 +427,19 @@ spec: targeted by the pool that have the CurrentMachineConfig as their config. type: integer format: int32 + certExpirys: + description: The certificate expiry dates from the controller config + type: array + items: + description: A label selector requirement is a selector that contains values, a key, and an operator that relates the key and values. + type: object + properties: + bundle: + description: the bundle for which the expiry applies + type: string + subject: + description: the subject of the cert + type: string + expiry: + description: the date when the cert expires + type: string diff --git a/manifests/controllerconfig.crd.yaml b/manifests/controllerconfig.crd.yaml index d6ae62b04f..bd1c119d44 100644 --- a/manifests/controllerconfig.crd.yaml +++ b/manifests/controllerconfig.crd.yaml @@ -1820,7 +1820,31 @@ spec: by the controller. format: int64 type: integer + controllerCertificates: + description: controllerCertificates holds the information about the MCCs certificates. + items: + description: ControllerCertificate contains certificate + information for ControllerConfigStatus + properties: + subject: + description: the subject of the cert. + nullable: true + type: string + signer: + description: signer contains the issuer of the cert + type: string + notBefore: + description: lower bound for validity + type: string + notAfter: + description: upper bound for validity + type: string + bundleFile: + description: the name of the bundle serving this cert. + type: string + type: object + type: array type: object required: - spec - type: object + type: object \ No newline at end of file diff --git a/pkg/apis/machineconfiguration.openshift.io/v1/types.go b/pkg/apis/machineconfiguration.openshift.io/v1/types.go index 8cfa3ba29d..1da8d49d7c 100644 --- a/pkg/apis/machineconfiguration.openshift.io/v1/types.go +++ b/pkg/apis/machineconfiguration.openshift.io/v1/types.go @@ -139,6 +139,28 @@ type ControllerConfigStatus struct { // conditions represents the latest available observations of current state. // +optional Conditions []ControllerConfigStatusCondition `json:"conditions"` + + // controllerCertificates represents the latest available observations of the automatically rotating certificates in the MCO. + // +optional + ControllerCertificates []ControllerCertificate `json:"controllerCertificates"` +} + +// ControllerCertificate contains info about a specific cert. +type ControllerCertificate struct { + // subject is the cert subject + Subject string `json:"subject"` + + // signer is the cert Issuer + Signer string `json:"signer"` + + // notBefore is the lower boundary for validity + NotBefore string `json:"notBefore"` + + // notAfter is the upper boundary for validity + NotAfter string `json:"notAfter"` + + // bundleFile is the larger bundle a cert comes from + BundleFile string `json:"bundleFile"` } // ControllerConfigStatusCondition contains condition information for ControllerConfigStatus @@ -303,6 +325,16 @@ type MachineConfigPoolStatus struct { // conditions represents the latest available observations of current state. // +optional Conditions []MachineConfigPoolCondition `json:"conditions"` + + // certExpirys keeps track of important certificate expiration data + CertExpirys []CertExpiry `json:"certExpirys"` +} + +// ceryExpiry contains the bundle name and the expiry date +type CertExpiry struct { + Bundle string `json:"bundle"` + Subject string `json:"subject"` + Expiry string `json:"expiry"` } // MachineConfigPoolStatusConfiguration stores the current configuration for the pool, and diff --git a/pkg/apis/machineconfiguration.openshift.io/v1/zz_generated.deepcopy.go b/pkg/apis/machineconfiguration.openshift.io/v1/zz_generated.deepcopy.go index 1e34177448..19ebc88102 100644 --- a/pkg/apis/machineconfiguration.openshift.io/v1/zz_generated.deepcopy.go +++ b/pkg/apis/machineconfiguration.openshift.io/v1/zz_generated.deepcopy.go @@ -292,6 +292,12 @@ func (in *ControllerConfigSpec) DeepCopy() *ControllerConfigSpec { return out } +func (in *ControllerCertificate) DeepCopyInto(out *ControllerCertificate) { + *out = *in + return + +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ControllerConfigStatus) DeepCopyInto(out *ControllerConfigStatus) { *out = *in @@ -302,6 +308,13 @@ func (in *ControllerConfigStatus) DeepCopyInto(out *ControllerConfigStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.ControllerCertificates != nil { + in, out := &in.ControllerCertificates, &out.ControllerCertificates + *out = make([]ControllerCertificate, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } @@ -332,6 +345,17 @@ func (in *ControllerConfigStatusCondition) DeepCopy() *ControllerConfigStatusCon return out } + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ControllerCertificate. +func (in *ControllerCertificate) DeepCopy() *ControllerCertificate { + if in == nil { + return nil + } + out := new(ControllerCertificate) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *KubeletConfig) DeepCopyInto(out *KubeletConfig) { *out = *in @@ -554,6 +578,23 @@ func (in *MachineConfigPool) DeepCopy() *MachineConfigPool { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CertExpiry) DeepCopyInto(out *CertExpiry) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CertExpiry. +func (in *CertExpiry) DeepCopy() *CertExpiry { + if in == nil { + return nil + } + out := new(CertExpiry) + in.DeepCopyInto(out) + return out +} + + // DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. func (in *MachineConfigPool) DeepCopyObject() runtime.Object { if c := in.DeepCopy(); c != nil { diff --git a/pkg/controller/node/node_controller_test.go b/pkg/controller/node/node_controller_test.go index 100340bef9..965a27b345 100644 --- a/pkg/controller/node/node_controller_test.go +++ b/pkg/controller/node/node_controller_test.go @@ -907,7 +907,7 @@ func TestShouldMakeProgress(t *testing.T) { } f.expectPatchNodeAction(expNode, exppatch) } - expStatus := calculateStatus(mcp, nodes) + expStatus := calculateStatus(cc, mcp, nodes) expMcp := mcp.DeepCopy() expMcp.Status = expStatus f.expectUpdateMachineConfigPoolStatus(expMcp) @@ -930,6 +930,7 @@ func TestEmptyCurrentMachineConfig(t *testing.T) { func TestPaused(t *testing.T) { f := newFixture(t) + cc := newControllerConfig(ctrlcommon.ControllerConfigName, configv1.TopologyMode("")) mcp := helpers.NewMachineConfigPool("test-cluster-infra", nil, helpers.InfraSelector, "v1") mcpWorker := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v1") mcp.Spec.MaxUnavailable = intStrPtr(intstr.FromInt(1)) @@ -939,6 +940,7 @@ func TestPaused(t *testing.T) { newNodeWithLabel("node-1", "v0", "v0", map[string]string{"node-role/worker": "", "node-role/infra": ""}), } + f.ccLister = append(f.ccLister, cc) f.mcpLister = append(f.mcpLister, mcp, mcpWorker) f.objects = append(f.objects, mcp, mcpWorker) f.nodeLister = append(f.nodeLister, nodes...) @@ -946,7 +948,7 @@ func TestPaused(t *testing.T) { f.kubeobjects = append(f.kubeobjects, nodes[idx]) } - expStatus := calculateStatus(mcp, nodes) + expStatus := calculateStatus(cc, mcp, nodes) expMcp := mcp.DeepCopy() expMcp.Status = expStatus f.expectUpdateMachineConfigPoolStatus(expMcp) @@ -973,7 +975,7 @@ func TestShouldUpdateStatusOnlyUpdated(t *testing.T) { f.kubeobjects = append(f.kubeobjects, nodes[idx]) } - expStatus := calculateStatus(mcp, nodes) + expStatus := calculateStatus(cc, mcp, nodes) expMcp := mcp.DeepCopy() expMcp.Status = expStatus f.expectUpdateMachineConfigPoolStatus(expMcp) @@ -1000,9 +1002,43 @@ func TestShouldUpdateStatusOnlyNoProgress(t *testing.T) { f.kubeobjects = append(f.kubeobjects, nodes[idx]) } - expStatus := calculateStatus(mcp, nodes) + expStatus := calculateStatus(cc, mcp, nodes) + expMcp := mcp.DeepCopy() + expMcp.Status = expStatus + f.expectUpdateMachineConfigPoolStatus(expMcp) + + f.run(getKey(mcp, t)) +} + +func TestCertStatus(t *testing.T) { + f := newFixture(t) + cc := newControllerConfig(ctrlcommon.ControllerConfigName, configv1.TopologyMode("")) + + cc.Status.ControllerCertificates = append(cc.Status.ControllerCertificates, mcfgv1.ControllerCertificate{ + BundleFile: "KubeAPIServerServingCAData", + NotAfter: time.Now().String(), + }) + + mcp := helpers.NewMachineConfigPool("test-cluster-infra", nil, helpers.InfraSelector, "v1") + mcpWorker := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v1") + mcp.Spec.MaxUnavailable = intStrPtr(intstr.FromInt(1)) + nodes := []*corev1.Node{ + newNodeWithLabel("node-0", "v1", "v1", map[string]string{"node-role/worker": "", "node-role/infra": ""}), + newNodeWithLabel("node-1", "v1", "v1", map[string]string{"node-role/worker": "", "node-role/infra": ""}), + } + + f.ccLister = append(f.ccLister, cc) + f.mcpLister = append(f.mcpLister, mcp, mcpWorker) + f.objects = append(f.objects, mcp, mcpWorker) + f.nodeLister = append(f.nodeLister, nodes...) + for idx := range nodes { + f.kubeobjects = append(f.kubeobjects, nodes[idx]) + } + + expStatus := calculateStatus(cc, mcp, nodes) expMcp := mcp.DeepCopy() expMcp.Status = expStatus + f.expectUpdateMachineConfigPoolStatus(expMcp) f.run(getKey(mcp, t)) @@ -1018,7 +1054,7 @@ func TestShouldDoNothing(t *testing.T) { newNodeWithLabel("node-0", "v1", "v1", map[string]string{"node-role/worker": "", "node-role/infra": ""}), newNodeWithLabel("node-1", "v1", "v1", map[string]string{"node-role/worker": "", "node-role/infra": ""}), } - status := calculateStatus(mcp, nodes) + status := calculateStatus(cc, mcp, nodes) mcp.Status = status f.ccLister = append(f.ccLister, cc) @@ -1107,7 +1143,7 @@ func TestControlPlaneTopology(t *testing.T) { for _, node := range nodes { addNodeAnnotations(node, annotations) } - status := calculateStatus(mcp, nodes) + status := calculateStatus(cc, mcp, nodes) mcp.Status = status f.ccLister = append(f.ccLister, cc) diff --git a/pkg/controller/node/status.go b/pkg/controller/node/status.go index 344d3dbe15..f1e31f472e 100644 --- a/pkg/controller/node/status.go +++ b/pkg/controller/node/status.go @@ -6,6 +6,8 @@ import ( "strings" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" + v1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" "github.com/openshift/machine-config-operator/pkg/daemon/constants" daemonconsts "github.com/openshift/machine-config-operator/pkg/daemon/constants" corev1 "k8s.io/api/core/v1" @@ -15,12 +17,16 @@ import ( ) func (ctrl *Controller) syncStatusOnly(pool *mcfgv1.MachineConfigPool) error { + cc, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName) + if err != nil { + return err + } nodes, err := ctrl.getNodesForPool(pool) if err != nil { return err } - newStatus := calculateStatus(pool, nodes) + newStatus := calculateStatus(cc, pool, nodes) if equality.Semantic.DeepEqual(pool.Status, newStatus) { return nil } @@ -37,7 +43,20 @@ func (ctrl *Controller) syncStatusOnly(pool *mcfgv1.MachineConfigPool) error { return err } -func calculateStatus(pool *mcfgv1.MachineConfigPool, nodes []*corev1.Node) mcfgv1.MachineConfigPoolStatus { +func calculateStatus(cconfig *v1.ControllerConfig, pool *mcfgv1.MachineConfigPool, nodes []*corev1.Node) mcfgv1.MachineConfigPoolStatus { + certExpirys := []v1.CertExpiry{} + if cconfig != nil { + for _, cert := range cconfig.Status.ControllerCertificates { + if cert.BundleFile == "KubeAPIServerServingCAData" { + certExpirys = append(certExpirys, v1.CertExpiry{ + Bundle: cert.BundleFile, + Subject: cert.Subject, + Expiry: cert.NotAfter, + }, + ) + } + } + } machineCount := int32(len(nodes)) updatedMachines := getUpdatedMachines(pool.Spec.Configuration.Name, nodes) @@ -66,6 +85,7 @@ func calculateStatus(pool *mcfgv1.MachineConfigPool, nodes []*corev1.Node) mcfgv ReadyMachineCount: readyMachineCount, UnavailableMachineCount: unavailableMachineCount, DegradedMachineCount: degradedMachineCount, + CertExpirys: certExpirys, } status.Configuration = pool.Status.Configuration diff --git a/pkg/controller/node/status_test.go b/pkg/controller/node/status_test.go index f634b522af..8f1d4afd43 100644 --- a/pkg/controller/node/status_test.go +++ b/pkg/controller/node/status_test.go @@ -641,7 +641,7 @@ func TestCalculateStatus(t *testing.T) { Paused: test.paused, }, } - status := calculateStatus(pool, test.nodes) + status := calculateStatus(nil, pool, test.nodes) test.verify(status, t) }) } diff --git a/pkg/controller/template/status.go b/pkg/controller/template/status.go index 18f99bd32e..af65099923 100644 --- a/pkg/controller/template/status.go +++ b/pkg/controller/template/status.go @@ -5,12 +5,12 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" "k8s.io/client-go/util/retry" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" mcfgclientv1 "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/typed/machineconfiguration.openshift.io/v1" "github.com/openshift/machine-config-operator/pkg/version" + "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -74,6 +74,15 @@ func (ctrl *Controller) syncCompletedStatus(ctrlconfig *mcfgv1.ControllerConfig) return updateControllerConfigStatus(ctrlconfig.GetName(), ctrl.ccLister.Get, ctrl.client.MachineconfigurationV1().ControllerConfigs(), updateFunc) } +// syncCertificateStatus places the new certitifcate data into the actual controllerConfig that is our source of truth. +func (ctrl *Controller) syncCertificateStatus(ctrlconfig *mcfgv1.ControllerConfig) error { + updateFunc := func(cfg *mcfgv1.ControllerConfig) error { + cfg.Status.ControllerCertificates = ctrlconfig.Status.ControllerCertificates + return nil + } + return updateControllerConfigStatus(ctrlconfig.GetName(), ctrl.ccLister.Get, ctrl.client.MachineconfigurationV1().ControllerConfigs(), updateFunc) +} + type updateControllerConfigStatusFunc func(*mcfgv1.ControllerConfig) error func updateControllerConfigStatus(name string, diff --git a/pkg/controller/template/template_controller.go b/pkg/controller/template/template_controller.go index 3eafa4197a..6bb01bedd2 100644 --- a/pkg/controller/template/template_controller.go +++ b/pkg/controller/template/template_controller.go @@ -3,7 +3,9 @@ package template import ( "bytes" "context" + "crypto/x509" "encoding/json" + "encoding/pem" "fmt" "reflect" "sort" @@ -13,6 +15,7 @@ import ( "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" mcoResourceApply "github.com/openshift/machine-config-operator/lib/resourceapply" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" + v1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" mcfgclientset "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned" "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/scheme" @@ -421,6 +424,45 @@ func (ctrl *Controller) handleErr(err error, key interface{}) { ctrl.queue.AddAfter(key, 1*time.Minute) } +// updateControllerConfigCerts parses the raw cert data and places key information about the certs into the controllerconfig status +func updateControllerConfigCerts(config *mcfgv1.ControllerConfig) bool { + modified := false + names := []string{ + "KubeAPIServerServingCAData", "CloudProviderCAData", "RootCAData", "AdditionalTrustBundle", + } + config.Status.ControllerCertificates = []v1.ControllerCertificate{} + certs := [][]byte{ + config.Spec.KubeAPIServerServingCAData, + config.Spec.CloudProviderCAData, + config.Spec.RootCAData, + config.Spec.AdditionalTrustBundle, + } + for i, cert := range certs { + for len(cert) > 0 { + b, next := pem.Decode(cert) + if b == nil { + break + } + c, err := x509.ParseCertificate(b.Bytes) + if err != nil { + klog.Infof("Malformed Cert, not syncing") + continue + } + cert = next + config.Status.ControllerCertificates = append(config.Status.ControllerCertificates, v1.ControllerCertificate{ + Subject: c.Subject.String(), + Signer: c.Issuer.String(), + NotBefore: c.NotBefore.String(), + NotAfter: c.NotAfter.String(), + BundleFile: names[i], + }) + modified = true + } + + } + return modified +} + // syncControllerConfig will sync the controller config with the given key. // This function is not meant to be invoked concurrently with the same key. func (ctrl *Controller) syncControllerConfig(key string) error { @@ -453,6 +495,15 @@ func (ctrl *Controller) syncControllerConfig(key string) error { } } + modified := updateControllerConfigCerts(cfg) + + if modified { + klog.Info("Detecting cert modification, syncing these changes to the true controllerConfig.") + if err := ctrl.syncCertificateStatus(cfg); err != nil { + return err + } + } + var pullSecretRaw []byte if cfg.Spec.PullSecret != nil { secret, err := ctrl.kubeClient.CoreV1().Secrets(cfg.Spec.PullSecret.Namespace).Get(context.TODO(), cfg.Spec.PullSecret.Name, metav1.GetOptions{}) diff --git a/test/e2e/mcd_test.go b/test/e2e/mcd_test.go index 61b9da5d8c..a30a085507 100644 --- a/test/e2e/mcd_test.go +++ b/test/e2e/mcd_test.go @@ -942,6 +942,15 @@ func TestMCDRotatesCertsOnPausedPool(t *testing.T) { t.Logf("Paused") // Rotate the certificates + controllerConfig, err := cs.ControllerConfigs().Get(context.TODO(), "machine-config-controller", metav1.GetOptions{}) + require.Nil(t, err) + + oldData := "" + for _, cert := range controllerConfig.Status.ControllerCertificates { + if cert.BundleFile == "KubeAPIServerServingCAData" { + oldData = cert.Subject + } + } t.Logf("Patching certificate") err = helpers.ForceKubeApiserverCertificateRotation(cs) require.Nil(t, err) @@ -957,7 +966,7 @@ func TestMCDRotatesCertsOnPausedPool(t *testing.T) { nodes, err := helpers.GetNodesByRole(cs, testPool) require.NotEmpty(t, nodes) selectedNode := nodes[0] - controllerConfig, err := cs.ControllerConfigs().Get(context.TODO(), "machine-config-controller", metav1.GetOptions{}) + controllerConfig, err = cs.ControllerConfigs().Get(context.TODO(), "machine-config-controller", metav1.GetOptions{}) require.Nil(t, err) err = helpers.WaitForMCDToSyncCert(t, cs, selectedNode, controllerConfig.ResourceVersion) require.Nil(t, err) @@ -993,6 +1002,9 @@ func TestMCDRotatesCertsOnPausedPool(t *testing.T) { err = helpers.WaitForPoolCompleteAny(t, cs, testPool) require.Nil(t, err) + err = helpers.WaitForCertStatusToChange(t, cs, oldData) + require.Nil(t, err) + } func createMCToAddFileForRole(name, role, filename, data string) *mcfgv1.MachineConfig { diff --git a/test/helpers/utils.go b/test/helpers/utils.go index 3e61f07b81..23a93caf8f 100644 --- a/test/helpers/utils.go +++ b/test/helpers/utils.go @@ -357,6 +357,34 @@ func GetMonitoringToken(t *testing.T, cs *framework.ClientSet) (string, error) { return token.Status.Token, nil } +// WaitForCertStatusToChange queries a controllerconfig until the ControllerCertificates changes. +func WaitForCertStatusToChange(t *testing.T, cs *framework.ClientSet, oldData string) error { + ctx := context.TODO() + if err := wait.PollUntilContextTimeout(ctx, 2*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { + controllerConfig, err := cs.ControllerConfigs().Get(context.TODO(), "machine-config-controller", metav1.GetOptions{}) + require.Nil(t, err) + + // shows us that the status is updating + + for _, cert := range controllerConfig.Status.ControllerCertificates { + t.Logf("comparing: %s to %s\n", oldData, cert.Subject) + t.Logf("also, other data: %s %s\n", cert.BundleFile, cert.Subject) + if cert.BundleFile == "KubeAPIServerServingCAData" { + if oldData != cert.Subject { + return true, nil + } + } + } + + return false, nil + }); err != nil { + return err + } + + return nil + +} + // WaitForMCDToSyncCert waits for the MCD to write annotation on the latest controllerconfig resourceVersion, // to indicate that is has completed the certificate write func WaitForMCDToSyncCert(t *testing.T, cs *framework.ClientSet, node corev1.Node, resourceVersion string) error { diff --git a/vendor/k8s.io/code-generator/generate-groups.sh b/vendor/k8s.io/code-generator/generate-groups.sh old mode 100644 new mode 100755 diff --git a/vendor/k8s.io/code-generator/generate-internal-groups.sh b/vendor/k8s.io/code-generator/generate-internal-groups.sh old mode 100644 new mode 100755 diff --git a/vendor/modules.txt b/vendor/modules.txt index 609340aa9a..8e75ec1ea6 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -423,8 +423,6 @@ github.com/gofrs/flock ## explicit; go 1.15 github.com/gogo/protobuf/proto github.com/gogo/protobuf/sortkeys -# github.com/golang/glog v1.0.0 -## explicit # github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da ## explicit github.com/golang/groupcache/lru