diff --git a/pkg/controller/common/helpers.go b/pkg/controller/common/helpers.go index a0fe7b1bce..847774ae89 100644 --- a/pkg/controller/common/helpers.go +++ b/pkg/controller/common/helpers.go @@ -15,7 +15,6 @@ import ( "os" "path/filepath" "reflect" - "sort" "strings" "text/template" @@ -759,7 +758,6 @@ func decompressPayload(r io.Reader) ([]byte, error) { // Units have one exception: dropins are concat'ed func removeIgnDuplicateFilesUnitsUsers(ignConfig ign2types.Config) (ign2types.Config, error) { - files := ignConfig.Storage.Files units := ignConfig.Systemd.Units users := ignConfig.Passwd.Users @@ -1238,7 +1236,6 @@ func (n namespacedEventRecorder) AnnotatedEventf(object runtime.Object, annotati func DoARebuild(pool *mcfgv1.MachineConfigPool) bool { _, ok := pool.Labels[RebuildPoolLabel] return ok - } // isSubdirectory checks if targetPath is a subdirectory of dirPath. @@ -1312,6 +1309,190 @@ func GetSecurityProfileCiphers(profile *configv1.TLSSecurityProfile) (string, [] return string(profileSpec.MinTLSVersion), crypto.OpenSSLToIANACipherSuites(profileSpec.Ciphers) } +// cipherComponents holds the Fedora crypto-policy component names that an +// OpenSSL cipher suite decomposes into. +type cipherComponents struct { + cipher string // e.g. "AES-256-GCM" + mac string // e.g. "AEAD", "SHA256" +} + +// opensslCipherInfo maps OpenSSL cipher suite names to their decomposed +// Fedora crypto-policy components. The source of truth for which OpenSSL +// cipher names exist is library-go's `openSSLToIANACiphers` map in +// github.com/openshift/library-go/pkg/crypto/crypto.go — this table must +// stay in sync with it. +var opensslCipherInfo = map[string]cipherComponents{ + // TLS 1.3 + "TLS_AES_128_GCM_SHA256": {cipher: "AES-128-GCM", mac: "AEAD"}, + "TLS_AES_256_GCM_SHA384": {cipher: "AES-256-GCM", mac: "AEAD"}, + "TLS_CHACHA20_POLY1305_SHA256": {cipher: "CHACHA20-POLY1305", mac: "AEAD"}, + // TLS 1.2 ECDHE GCM + "ECDHE-ECDSA-AES128-GCM-SHA256": {cipher: "AES-128-GCM", mac: "AEAD"}, + "ECDHE-RSA-AES128-GCM-SHA256": {cipher: "AES-128-GCM", mac: "AEAD"}, + "ECDHE-ECDSA-AES256-GCM-SHA384": {cipher: "AES-256-GCM", mac: "AEAD"}, + "ECDHE-RSA-AES256-GCM-SHA384": {cipher: "AES-256-GCM", mac: "AEAD"}, + // TLS 1.2 ECDHE ChaCha20 + "ECDHE-ECDSA-CHACHA20-POLY1305": {cipher: "CHACHA20-POLY1305", mac: "AEAD"}, + "ECDHE-RSA-CHACHA20-POLY1305": {cipher: "CHACHA20-POLY1305", mac: "AEAD"}, + // TLS 1.2 ECDHE CBC (SHA256/SHA384 variants require TLS 1.2 PRF) + "ECDHE-ECDSA-AES128-SHA256": {cipher: "AES-128-CBC", mac: "HMAC-SHA2-256"}, + "ECDHE-RSA-AES128-SHA256": {cipher: "AES-128-CBC", mac: "HMAC-SHA2-256"}, + "ECDHE-ECDSA-AES256-SHA384": {cipher: "AES-256-CBC", mac: "HMAC-SHA2-384"}, // from ciphersUnsupportedByGo + "ECDHE-RSA-AES256-SHA384": {cipher: "AES-256-CBC", mac: "HMAC-SHA2-384"}, // from ciphersUnsupportedByGo + // TLS 1.0 ECDHE CBC (predate TLS 1.2 but usable with it) + "ECDHE-ECDSA-AES128-SHA": {cipher: "AES-128-CBC", mac: "HMAC-SHA1"}, + "ECDHE-RSA-AES128-SHA": {cipher: "AES-128-CBC", mac: "HMAC-SHA1"}, + "ECDHE-ECDSA-AES256-SHA": {cipher: "AES-256-CBC", mac: "HMAC-SHA1"}, + "ECDHE-RSA-AES256-SHA": {cipher: "AES-256-CBC", mac: "HMAC-SHA1"}, + // TLS 1.2 RSA key exchange + "AES128-GCM-SHA256": {cipher: "AES-128-GCM", mac: "AEAD"}, + "AES256-GCM-SHA384": {cipher: "AES-256-GCM", mac: "AEAD"}, + "AES128-SHA256": {cipher: "AES-128-CBC", mac: "HMAC-SHA2-256"}, + "AES256-SHA256": {cipher: "AES-256-CBC", mac: "HMAC-SHA2-256"}, // from ciphersUnsupportedByGo + // TLS 1.0 RSA key exchange (predate TLS 1.2 but usable with it) + "AES128-SHA": {cipher: "AES-128-CBC", mac: "HMAC-SHA1"}, + "AES256-SHA": {cipher: "AES-256-CBC", mac: "HMAC-SHA1"}, + // Legacy (3DES is removed from OpenSSL on RHCOS but harmless in the .pmod) + "DES-CBC3-SHA": {cipher: "3DES-CBC", mac: "HMAC-SHA1"}, + "ECDHE-RSA-DES-CBC3-SHA": {cipher: "3DES-CBC", mac: "HMAC-SHA1"}, +} + +// protocolVersionsBelowMinimum maps a TLS version to the protocol versions +// that must be removed from the base policy via subtractive syntax (-VERSION). +// This approach preserves any protocol versions in the base policy that are at +// or above the minimum (including DTLS peers), and automatically inherits new +// versions (e.g. DTLS1.3) when they appear in future base policies. +// TLS 1.0 and 1.1 are clamped to TLS 1.2 because RHCOS cannot deliver them: +// OpenSSL 3.x enforces @SECLEVEL=2 which forbids TLS < 1.2. +var protocolVersionsBelowMinimum = map[configv1.TLSProtocolVersion][]string{ + configv1.VersionTLS10: {"TLS1.0", "TLS1.1", "DTLS1.0"}, + configv1.VersionTLS11: {"TLS1.0", "TLS1.1", "DTLS1.0"}, + configv1.VersionTLS12: {"TLS1.0", "TLS1.1", "DTLS1.0"}, + configv1.VersionTLS13: {"TLS1.0", "TLS1.1", "TLS1.2", "DTLS1.0", "DTLS1.2"}, +} + +// tlsVersionsClamped contains TLS versions that are requested but cannot be +// delivered on RHCOS, used to emit a warning log. +var tlsVersionsClamped = map[configv1.TLSProtocolVersion]bool{ + configv1.VersionTLS10: true, + configv1.VersionTLS11: true, +} + +// tlsGroupToCryptoPolicy maps OpenShift TLSGroup enum values to Fedora +// crypto-policy group names. The canonical names are in the .pol files at +// https://gitlab.com/redhat-crypto/fedora-crypto-policies/-/tree/master/policies +var tlsGroupToCryptoPolicy = map[configv1.TLSGroup]string{ + configv1.TLSGroupX25519: "X25519", + configv1.TLSGroupSecP256r1: "SECP256R1", + configv1.TLSGroupSecP384r1: "SECP384R1", + configv1.TLSGroupSecP521r1: "SECP521R1", + configv1.TLSGroupX25519MLKEM768: "MLKEM768-X25519", + configv1.TLSGroupSecP256r1MLKEM768: "P256-MLKEM768", + configv1.TLSGroupSecP384r1MLKEM1024: "P384-MLKEM1024", +} + +// buildCustomSubPolicy generates the content of a .pmod file from a custom +// TLS profile spec. Cipher, MAC, and group directives use override syntax; +// protocol uses subtractive syntax (-VERSION) to preserve base policy DTLS. +func buildCustomSubPolicy(spec *configv1.TLSProfileSpec) string { + cipherSet := make(map[string]struct{}) + macSet := make(map[string]struct{}) + + for _, c := range spec.Ciphers { + info, ok := opensslCipherInfo[c] + if !ok { + klog.Warningf("Unknown cipher %q in custom TLS profile, skipping crypto-policy mapping", c) + continue + } + cipherSet[info.cipher] = struct{}{} + macSet[info.mac] = struct{}{} + } + + var lines []string + + if len(cipherSet) > 0 { + lines = append(lines, "cipher@TLS = "+sortedKeys(cipherSet)) + } + if len(macSet) > 0 { + lines = append(lines, "mac@TLS = "+sortedKeys(macSet)) + } + if toRemove, ok := protocolVersionsBelowMinimum[spec.MinTLSVersion]; ok { + if tlsVersionsClamped[spec.MinTLSVersion] { + klog.Warningf("TLS profile requests %s but RHCOS enforces TLS 1.2 minimum; clamping protocol@TLS to TLS1.2+", spec.MinTLSVersion) + } + var removals []string + for _, v := range toRemove { + removals = append(removals, "-"+v) + } + lines = append(lines, "protocol@TLS = "+strings.Join(removals, " ")) + } + if len(spec.Groups) > 0 { + var groups []string + for _, g := range spec.Groups { + if cpName, ok := tlsGroupToCryptoPolicy[g]; ok { + groups = append(groups, cpName) + } + } + if len(groups) > 0 { + lines = append(lines, "group@TLS = "+strings.Join(groups, " ")) + } + } + + return strings.Join(lines, "\n") +} + +func sortedKeys(m map[string]struct{}) string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return strings.Join(keys, " ") +} + +const ( + cryptoPolicyDefault = "DEFAULT" + cryptoPolicyDefaultOpenShift = "DEFAULT:OPENSHIFT" + cryptoPolicyLegacy = "LEGACY" + cryptoPolicyLegacyOpenShift = "LEGACY:OPENSHIFT" +) + +// GetCryptoPolicyFromTLSProfile maps an OpenShift TLS security profile to a +// Fedora crypto-policy name and optional sub-policy module content. +func GetCryptoPolicyFromTLSProfile(profile *configv1.TLSSecurityProfile) (string, string) { + profileType := configv1.TLSProfileIntermediateType + if profile != nil { + profileType = profile.Type + } + + switch profileType { + case configv1.TLSProfileModernType: + spec := configv1.TLSProfiles[configv1.TLSProfileModernType] + content := buildCustomSubPolicy(spec) + if content != "" { + return cryptoPolicyDefaultOpenShift, content + } + return cryptoPolicyDefault, "" + case configv1.TLSProfileOldType: + spec := configv1.TLSProfiles[configv1.TLSProfileOldType] + content := buildCustomSubPolicy(spec) + if content != "" { + return cryptoPolicyLegacyOpenShift, content + } + return cryptoPolicyLegacy, "" + case configv1.TLSProfileCustomType: + if profile.Custom != nil { + content := buildCustomSubPolicy(&profile.Custom.TLSProfileSpec) + if content != "" { + return cryptoPolicyDefaultOpenShift, content + } + } + return cryptoPolicyDefault, "" + default: + return cryptoPolicyDefault, "" + } +} + // Converts tlsMinVersion and tlscipherSuites flags to a tlsConfig object that is used // by the http.Server() call used in apiserver.NewAPIServer() & apiserver.Serve() // diff --git a/pkg/controller/common/helpers_test.go b/pkg/controller/common/helpers_test.go index 0cab9a8565..78571659db 100644 --- a/pkg/controller/common/helpers_test.go +++ b/pkg/controller/common/helpers_test.go @@ -19,6 +19,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + configv1 "github.com/openshift/api/config/v1" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" "github.com/openshift/client-go/machineconfiguration/clientset/versioned/fake" informers "github.com/openshift/client-go/machineconfiguration/informers/externalversions" @@ -91,8 +92,10 @@ func TestValidateIgnition(t *testing.T) { // Test that a valid ignition config returns nil testIgn2Config.Ignition.Version = "2.0.0" ign2Mode := 420 - ign2File := ign2types.File{Node: ign2types.Node{Filesystem: "root", Path: "/etc/testfileconfig"}, - FileEmbedded1: ign2types.FileEmbedded1{Mode: &ign2Mode, Contents: ign2types.FileContents{Source: "data:,helloworld"}}} + ign2File := ign2types.File{ + Node: ign2types.Node{Filesystem: "root", Path: "/etc/testfileconfig"}, + FileEmbedded1: ign2types.FileEmbedded1{Mode: &ign2Mode, Contents: ign2types.FileContents{Source: "data:,helloworld"}}, + } testIgn2Config.Storage.Files = []ign2types.File{ign2File} isValid = ValidateIgnition(testIgn2Config) require.Nil(t, isValid) @@ -112,8 +115,10 @@ func TestValidateIgnition(t *testing.T) { testIgn3Config.Ignition.Version = InternalMCOIgnitionVersion mode := 420 testfiledata := "data:,greatconfigstuff" - tempFile := ign3types.File{Node: ign3types.Node{Path: "/etc/testfileconfig"}, - FileEmbedded1: ign3types.FileEmbedded1{Contents: ign3types.Resource{Source: &testfiledata}, Mode: &mode}} + tempFile := ign3types.File{ + Node: ign3types.Node{Path: "/etc/testfileconfig"}, + FileEmbedded1: ign3types.FileEmbedded1{Contents: ign3types.Resource{Source: &testfiledata}, Mode: &mode}, + } testIgn3Config.Storage.Files = append(testIgn3Config.Storage.Files, tempFile) isValid2 = ValidateIgnition(testIgn3Config) require.Nil(t, isValid2) @@ -232,7 +237,8 @@ func TestIgnitionConverterConvert(t *testing.T) { inputVersion: "3.5.0", outputVersion: "3.1.0", err: ErrIgnitionConverterWrongSourceType, - }, { + }, + { name: "Conversion not supported", inputConfig: ign3Config, inputVersion: "3.1.0", @@ -259,7 +265,6 @@ func TestIgnitionConverterConvert(t *testing.T) { } else { assert.ErrorIs(t, err, testCase.err) } - }) } } @@ -643,7 +648,6 @@ func TestMergeMachineConfigs(t *testing.T) { }, } assert.Equal(t, *mergedMachineConfig, *expectedMachineConfig) - } func TestRemoveIgnDuplicateFilesAndUnits(t *testing.T) { @@ -761,12 +765,18 @@ func TestSetDefaultFileOverwrite(t *testing.T) { // Set up Files entries mode := 420 testfiledata := "data:,test" - tempFileNoDefault := ign3types.File{Node: ign3types.Node{Path: "/etc/testfileconfig1"}, - FileEmbedded1: ign3types.FileEmbedded1{Contents: ign3types.Resource{Source: &testfiledata}, Mode: &mode}} - tempFileOvewriteTrue := ign3types.File{Node: ign3types.Node{Path: "/etc/testfileconfig1", Overwrite: boolToPtr(true)}, - FileEmbedded1: ign3types.FileEmbedded1{Contents: ign3types.Resource{Source: &testfiledata}, Mode: &mode}} - tempFileOverwriteFalse := ign3types.File{Node: ign3types.Node{Path: "/etc/testfileconfig2", Overwrite: boolToPtr(false)}, - FileEmbedded1: ign3types.FileEmbedded1{Contents: ign3types.Resource{Source: &testfiledata}, Mode: &mode}} + tempFileNoDefault := ign3types.File{ + Node: ign3types.Node{Path: "/etc/testfileconfig1"}, + FileEmbedded1: ign3types.FileEmbedded1{Contents: ign3types.Resource{Source: &testfiledata}, Mode: &mode}, + } + tempFileOvewriteTrue := ign3types.File{ + Node: ign3types.Node{Path: "/etc/testfileconfig1", Overwrite: boolToPtr(true)}, + FileEmbedded1: ign3types.FileEmbedded1{Contents: ign3types.Resource{Source: &testfiledata}, Mode: &mode}, + } + tempFileOverwriteFalse := ign3types.File{ + Node: ign3types.Node{Path: "/etc/testfileconfig2", Overwrite: boolToPtr(false)}, + FileEmbedded1: ign3types.FileEmbedded1{Contents: ign3types.Resource{Source: &testfiledata}, Mode: &mode}, + } // Set up two Ignition configs, one with overwrite: no default, overwrite: false (to be passed to MergeMachineConfigs) // and one with a overwrite: true, overwrite: false (the expected output) @@ -822,8 +832,10 @@ func TestIgnitionMergeCompressed(t *testing.T) { mode := 420 testfiledata := "data:;base64,H4sIAAAAAAAAA0vLz+cCAKhlMn4EAAAA" compression := "gzip" - tempFile := ign3types.File{Node: ign3types.Node{Path: "/etc/testfileconfig"}, - FileEmbedded1: ign3types.FileEmbedded1{Contents: ign3types.Resource{Source: &testfiledata, Compression: &compression}, Mode: &mode}} + tempFile := ign3types.File{ + Node: ign3types.Node{Path: "/etc/testfileconfig"}, + FileEmbedded1: ign3types.FileEmbedded1{Contents: ign3types.Resource{Source: &testfiledata, Compression: &compression}, Mode: &mode}, + } testIgn3Config.Storage.Files = append(testIgn3Config.Storage.Files, tempFile) testIgn3Config2 := ign3types.Config{} @@ -1016,292 +1028,293 @@ func TestGetMachinesByState(t *testing.T) { layered bool mosc *mcfgv1.MachineOSConfig mosb *mcfgv1.MachineOSBuild - }{{ - name: "no nodes", - nodes: []*corev1.Node{}, - currentConfig: machineConfigV1, - }, { - name: "node with nil annotations", - nodes: []*corev1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node1", - Annotations: nil, + }{ + { + name: "no nodes", + nodes: []*corev1.Node{}, + currentConfig: machineConfigV1, + }, { + name: "node with nil annotations", + nodes: []*corev1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + Annotations: nil, + }, }, }, - }, - unavailable: []*corev1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node1", - Annotations: nil, + unavailable: []*corev1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + Annotations: nil, + }, }, }, - }, - }, { - name: "node with empty annotations", - nodes: []*corev1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node1", - Annotations: map[string]string{}, + }, { + name: "node with empty annotations", + nodes: []*corev1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + Annotations: map[string]string{}, + }, }, }, - }, - unavailable: []*corev1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node1", - Annotations: map[string]string{}, + unavailable: []*corev1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + Annotations: map[string]string{}, + }, }, }, + }, { + name: "1 node updated, 1 node ready, 1 updating, 1 not acted upon, 0 degraded", + nodes: []*corev1.Node{ + newNode(machineConfigV0, machineConfigV0), + newNode(machineConfigV1, machineConfigV1), + newNode(machineConfigV0, machineConfigV1), + }, + currentConfig: machineConfigV1, + updated: []*corev1.Node{newNode(machineConfigV1, machineConfigV1)}, + ready: []*corev1.Node{newNode(machineConfigV1, machineConfigV1)}, + unavailable: []*corev1.Node{newNode(machineConfigV0, machineConfigV1)}, + }, { + name: "2 node updated, 1 updating", + nodes: []*corev1.Node{ + newNode(machineConfigV0, machineConfigV1), + newNode(machineConfigV1, machineConfigV1), + newNode(machineConfigV1, machineConfigV1), + }, + currentConfig: machineConfigV1, + updated: []*corev1.Node{newNode(machineConfigV1, machineConfigV1), newNode(machineConfigV1, machineConfigV1)}, + ready: []*corev1.Node{newNode(machineConfigV1, machineConfigV1), newNode(machineConfigV1, machineConfigV1)}, + unavailable: []*corev1.Node{newNode(machineConfigV0, machineConfigV1)}, + }, { + name: "2 node updated, 1 updating, but 1 updated node is NotReady", + nodes: []*corev1.Node{ + newNode(machineConfigV0, machineConfigV1), + newNode(machineConfigV1, machineConfigV1), + helpers.NewNodeWithReady("node-2", machineConfigV1, machineConfigV1, corev1.ConditionFalse), + }, + currentConfig: machineConfigV1, + updated: []*corev1.Node{newNode(machineConfigV1, machineConfigV1), helpers.NewNodeWithReady("node-2", machineConfigV1, machineConfigV1, corev1.ConditionFalse)}, + ready: []*corev1.Node{newNode(machineConfigV1, machineConfigV1)}, + unavailable: []*corev1.Node{newNode(machineConfigV0, machineConfigV1), helpers.NewNodeWithReady("node-2", machineConfigV1, machineConfigV1, corev1.ConditionFalse)}, + }, { + name: "1 layered node updated, 1 updating, 1 not acted upon", + nodes: []*corev1.Node{ + newLayeredNode(machineConfigV0, machineConfigV0, imageV0, imageV0), + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + newLayeredNode(machineConfigV0, machineConfigV1, imageV0, imageV1), + }, + currentConfig: machineConfigV1, + currentImage: imageV1, + layered: true, + mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), + mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV1).MachineOSBuild(), + updated: []*corev1.Node{newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1)}, + ready: []*corev1.Node{newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1)}, + unavailable: []*corev1.Node{newLayeredNode(machineConfigV0, machineConfigV1, imageV0, imageV1)}, + }, { + name: "2 layered nodes updated, 1 updating MachineConfig", + nodes: []*corev1.Node{ + newLayeredNode(machineConfigV0, machineConfigV1, imageV1, imageV1), + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + }, + currentConfig: machineConfigV1, + currentImage: imageV1, + layered: true, + mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), + mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV1).MachineOSBuild(), + updated: []*corev1.Node{ + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + }, + ready: []*corev1.Node{ + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + }, + unavailable: []*corev1.Node{newLayeredNode(machineConfigV0, machineConfigV1, imageV1, imageV1)}, + }, { + name: "2 layered nodes updated, 1 updating image", + nodes: []*corev1.Node{ + newLayeredNode(machineConfigV1, machineConfigV1, imageV0, imageV1), + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + }, + currentConfig: machineConfigV1, + currentImage: imageV1, + layered: true, + mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), + mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV1).MachineOSBuild(), + updated: []*corev1.Node{ + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + }, + ready: []*corev1.Node{ + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + }, + unavailable: []*corev1.Node{newLayeredNode(machineConfigV1, machineConfigV1, imageV0, imageV1)}, + }, { + name: "2 layered nodes updated, 1 updating, but 1 updated node is NotReady", + nodes: []*corev1.Node{ + newLayeredNode(machineConfigV0, machineConfigV1, imageV0, imageV1), + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + helpers.NewLayeredNodeWithReady("node-2", machineConfigV1, machineConfigV1, imageV1, imageV1, corev1.ConditionFalse), + }, + currentConfig: machineConfigV1, + currentImage: imageV1, + layered: true, + mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), + mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV1).MachineOSBuild(), + updated: []*corev1.Node{ + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + helpers.NewLayeredNodeWithReady("node-2", machineConfigV1, machineConfigV1, imageV1, imageV1, corev1.ConditionFalse), + }, + ready: []*corev1.Node{ + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + }, + unavailable: []*corev1.Node{ + newLayeredNode(machineConfigV0, machineConfigV1, imageV0, imageV1), + helpers.NewLayeredNodeWithReady("node-2", machineConfigV1, machineConfigV1, imageV1, imageV1, corev1.ConditionFalse), + }, + }, { + name: "Layered pool with unlayered nodes, 2 updated, 1 not layered and not updating", + nodes: []*corev1.Node{ + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + newNode(machineConfigV0, machineConfigV0), + }, + currentConfig: machineConfigV1, + currentImage: imageV1, + layered: true, + mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), + mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV1).MachineOSBuild(), + updated: []*corev1.Node{ + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + }, + ready: []*corev1.Node{ + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), + }, + }, { + name: "Layered pool with image not built, 3 not updated or ready, 0 updating", + nodes: []*corev1.Node{ + newNode(machineConfigV1, machineConfigV1), + newNode(machineConfigV1, machineConfigV1), + newNode(machineConfigV1, machineConfigV1), + }, + currentConfig: machineConfigV1, + layered: true, + mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithMachineConfigPool("pool-1").MachineOSConfig(), + mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV1).MachineOSBuild(), + }, { + name: "Unlayered pool with 1 layered node, 2 updated, 1 not acted upon", + nodes: []*corev1.Node{ + newNode(machineConfigV1, machineConfigV1), + newNode(machineConfigV1, machineConfigV1), + newLayeredNode(machineConfigV0, machineConfigV0, imageV1, imageV1), + }, + currentConfig: machineConfigV1, + updated: []*corev1.Node{ + newNode(machineConfigV1, machineConfigV1), + newNode(machineConfigV1, machineConfigV1), + }, + ready: []*corev1.Node{ + newNode(machineConfigV1, machineConfigV1), + newNode(machineConfigV1, machineConfigV1), + }, + }, { + name: "Pool with image mode disabling, 1 node updating, 2 nodes not acted upon", + nodes: []*corev1.Node{ + newLayeredNode(machineConfigV0, machineConfigV0, imageV1, ""), + newLayeredNode(machineConfigV0, machineConfigV0, imageV1, imageV1), + newLayeredNode(machineConfigV0, machineConfigV0, imageV1, imageV1), + }, + currentConfig: machineConfigV0, + layered: false, + unavailable: []*corev1.Node{ + newLayeredNode(machineConfigV0, machineConfigV0, imageV1, ""), + }, + }, { + name: "Nodes with mixed states", + nodes: []*corev1.Node{ + newNodeWithState(machineConfigV0, machineConfigV0, "Degraded"), + newNodeWithState(machineConfigV0, machineConfigV0, "Done"), + newNodeWithState(machineConfigV0, machineConfigV0, "Unreconcilable"), + newNodeWithState(machineConfigV0, machineConfigV1, "Working"), + newNodeWithState(machineConfigV0, machineConfigV1, "Rebooting"), + }, + currentConfig: machineConfigV0, + updated: []*corev1.Node{ + newNodeWithState(machineConfigV0, machineConfigV0, "Done"), + }, + ready: []*corev1.Node{ + newNodeWithState(machineConfigV0, machineConfigV0, "Done"), + }, + unavailable: []*corev1.Node{ + newNodeWithState(machineConfigV0, machineConfigV0, "Degraded"), + newNodeWithState(machineConfigV0, machineConfigV0, "Unreconcilable"), + newNodeWithState(machineConfigV0, machineConfigV1, "Working"), + newNodeWithState(machineConfigV0, machineConfigV1, "Rebooting"), + }, + degraded: []*corev1.Node{ + newNodeWithState(machineConfigV0, machineConfigV0, "Degraded"), + newNodeWithState(machineConfigV0, machineConfigV0, "Unreconcilable"), + }, + }, { + name: "Layered nodes with mixed states", + nodes: []*corev1.Node{ + newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Degraded"), + newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Done"), + newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Unreconcilable"), + newLayeredNodeWithState(machineConfigV0, machineConfigV1, imageV1, imageV1, "Working"), + newLayeredNodeWithState(machineConfigV0, machineConfigV1, imageV1, imageV1, "Rebooting"), + }, + currentConfig: machineConfigV0, + currentImage: imageV1, + layered: true, + mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), + mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV0).MachineOSBuild(), + updated: []*corev1.Node{ + newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Done"), + }, + ready: []*corev1.Node{ + newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Done"), + }, + unavailable: []*corev1.Node{ + newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Degraded"), + newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Unreconcilable"), + newLayeredNodeWithState(machineConfigV0, machineConfigV1, imageV1, imageV1, "Working"), + newLayeredNodeWithState(machineConfigV0, machineConfigV1, imageV1, imageV1, "Rebooting"), + }, + degraded: []*corev1.Node{ + newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Degraded"), + newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Unreconcilable"), + }, + }, { + name: "0 layered nodes updated, 2 degraded", + nodes: []*corev1.Node{ + newLayeredNodeWithState(machineConfigV1, machineConfigV0, imageV1, imageV1, "Degraded"), + newLayeredNodeWithState(machineConfigV1, machineConfigV0, imageV1, imageV1, "Unreconcilable"), + newLayeredNode(machineConfigV0, machineConfigV0, imageV1, imageV1), + newLayeredNode(machineConfigV0, machineConfigV0, imageV1, imageV1), + }, + currentConfig: machineConfigV1, + currentImage: imageV1, + layered: true, + mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), + mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV0).MachineOSBuild(), + degraded: []*corev1.Node{ + newLayeredNodeWithState(machineConfigV1, machineConfigV0, imageV1, imageV1, "Degraded"), + newLayeredNodeWithState(machineConfigV1, machineConfigV0, imageV1, imageV1, "Unreconcilable"), + }, }, - }, { - name: "1 node updated, 1 node ready, 1 updating, 1 not acted upon, 0 degraded", - nodes: []*corev1.Node{ - newNode(machineConfigV0, machineConfigV0), - newNode(machineConfigV1, machineConfigV1), - newNode(machineConfigV0, machineConfigV1), - }, - currentConfig: machineConfigV1, - updated: []*corev1.Node{newNode(machineConfigV1, machineConfigV1)}, - ready: []*corev1.Node{newNode(machineConfigV1, machineConfigV1)}, - unavailable: []*corev1.Node{newNode(machineConfigV0, machineConfigV1)}, - }, { - name: "2 node updated, 1 updating", - nodes: []*corev1.Node{ - newNode(machineConfigV0, machineConfigV1), - newNode(machineConfigV1, machineConfigV1), - newNode(machineConfigV1, machineConfigV1), - }, - currentConfig: machineConfigV1, - updated: []*corev1.Node{newNode(machineConfigV1, machineConfigV1), newNode(machineConfigV1, machineConfigV1)}, - ready: []*corev1.Node{newNode(machineConfigV1, machineConfigV1), newNode(machineConfigV1, machineConfigV1)}, - unavailable: []*corev1.Node{newNode(machineConfigV0, machineConfigV1)}, - }, { - name: "2 node updated, 1 updating, but 1 updated node is NotReady", - nodes: []*corev1.Node{ - newNode(machineConfigV0, machineConfigV1), - newNode(machineConfigV1, machineConfigV1), - helpers.NewNodeWithReady("node-2", machineConfigV1, machineConfigV1, corev1.ConditionFalse), - }, - currentConfig: machineConfigV1, - updated: []*corev1.Node{newNode(machineConfigV1, machineConfigV1), helpers.NewNodeWithReady("node-2", machineConfigV1, machineConfigV1, corev1.ConditionFalse)}, - ready: []*corev1.Node{newNode(machineConfigV1, machineConfigV1)}, - unavailable: []*corev1.Node{newNode(machineConfigV0, machineConfigV1), helpers.NewNodeWithReady("node-2", machineConfigV1, machineConfigV1, corev1.ConditionFalse)}, - }, { - name: "1 layered node updated, 1 updating, 1 not acted upon", - nodes: []*corev1.Node{ - newLayeredNode(machineConfigV0, machineConfigV0, imageV0, imageV0), - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - newLayeredNode(machineConfigV0, machineConfigV1, imageV0, imageV1), - }, - currentConfig: machineConfigV1, - currentImage: imageV1, - layered: true, - mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), - mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV1).MachineOSBuild(), - updated: []*corev1.Node{newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1)}, - ready: []*corev1.Node{newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1)}, - unavailable: []*corev1.Node{newLayeredNode(machineConfigV0, machineConfigV1, imageV0, imageV1)}, - }, { - name: "2 layered nodes updated, 1 updating MachineConfig", - nodes: []*corev1.Node{ - newLayeredNode(machineConfigV0, machineConfigV1, imageV1, imageV1), - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - }, - currentConfig: machineConfigV1, - currentImage: imageV1, - layered: true, - mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), - mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV1).MachineOSBuild(), - updated: []*corev1.Node{ - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - }, - ready: []*corev1.Node{ - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - }, - unavailable: []*corev1.Node{newLayeredNode(machineConfigV0, machineConfigV1, imageV1, imageV1)}, - }, { - name: "2 layered nodes updated, 1 updating image", - nodes: []*corev1.Node{ - newLayeredNode(machineConfigV1, machineConfigV1, imageV0, imageV1), - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - }, - currentConfig: machineConfigV1, - currentImage: imageV1, - layered: true, - mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), - mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV1).MachineOSBuild(), - updated: []*corev1.Node{ - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - }, - ready: []*corev1.Node{ - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - }, - unavailable: []*corev1.Node{newLayeredNode(machineConfigV1, machineConfigV1, imageV0, imageV1)}, - }, { - name: "2 layered nodes updated, 1 updating, but 1 updated node is NotReady", - nodes: []*corev1.Node{ - newLayeredNode(machineConfigV0, machineConfigV1, imageV0, imageV1), - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - helpers.NewLayeredNodeWithReady("node-2", machineConfigV1, machineConfigV1, imageV1, imageV1, corev1.ConditionFalse), - }, - currentConfig: machineConfigV1, - currentImage: imageV1, - layered: true, - mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), - mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV1).MachineOSBuild(), - updated: []*corev1.Node{ - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - helpers.NewLayeredNodeWithReady("node-2", machineConfigV1, machineConfigV1, imageV1, imageV1, corev1.ConditionFalse), - }, - ready: []*corev1.Node{ - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - }, - unavailable: []*corev1.Node{ - newLayeredNode(machineConfigV0, machineConfigV1, imageV0, imageV1), - helpers.NewLayeredNodeWithReady("node-2", machineConfigV1, machineConfigV1, imageV1, imageV1, corev1.ConditionFalse), - }, - }, { - name: "Layered pool with unlayered nodes, 2 updated, 1 not layered and not updating", - nodes: []*corev1.Node{ - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - newNode(machineConfigV0, machineConfigV0), - }, - currentConfig: machineConfigV1, - currentImage: imageV1, - layered: true, - mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), - mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV1).MachineOSBuild(), - updated: []*corev1.Node{ - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - }, - ready: []*corev1.Node{ - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - newLayeredNode(machineConfigV1, machineConfigV1, imageV1, imageV1), - }, - }, { - name: "Layered pool with image not built, 3 not updated or ready, 0 updating", - nodes: []*corev1.Node{ - newNode(machineConfigV1, machineConfigV1), - newNode(machineConfigV1, machineConfigV1), - newNode(machineConfigV1, machineConfigV1), - }, - currentConfig: machineConfigV1, - layered: true, - mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithMachineConfigPool("pool-1").MachineOSConfig(), - mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV1).MachineOSBuild(), - }, { - name: "Unlayered pool with 1 layered node, 2 updated, 1 not acted upon", - nodes: []*corev1.Node{ - newNode(machineConfigV1, machineConfigV1), - newNode(machineConfigV1, machineConfigV1), - newLayeredNode(machineConfigV0, machineConfigV0, imageV1, imageV1), - }, - currentConfig: machineConfigV1, - updated: []*corev1.Node{ - newNode(machineConfigV1, machineConfigV1), - newNode(machineConfigV1, machineConfigV1), - }, - ready: []*corev1.Node{ - newNode(machineConfigV1, machineConfigV1), - newNode(machineConfigV1, machineConfigV1), - }, - }, { - name: "Pool with image mode disabling, 1 node updating, 2 nodes not acted upon", - nodes: []*corev1.Node{ - newLayeredNode(machineConfigV0, machineConfigV0, imageV1, ""), - newLayeredNode(machineConfigV0, machineConfigV0, imageV1, imageV1), - newLayeredNode(machineConfigV0, machineConfigV0, imageV1, imageV1), - }, - currentConfig: machineConfigV0, - layered: false, - unavailable: []*corev1.Node{ - newLayeredNode(machineConfigV0, machineConfigV0, imageV1, ""), - }, - }, { - name: "Nodes with mixed states", - nodes: []*corev1.Node{ - newNodeWithState(machineConfigV0, machineConfigV0, "Degraded"), - newNodeWithState(machineConfigV0, machineConfigV0, "Done"), - newNodeWithState(machineConfigV0, machineConfigV0, "Unreconcilable"), - newNodeWithState(machineConfigV0, machineConfigV1, "Working"), - newNodeWithState(machineConfigV0, machineConfigV1, "Rebooting"), - }, - currentConfig: machineConfigV0, - updated: []*corev1.Node{ - newNodeWithState(machineConfigV0, machineConfigV0, "Done"), - }, - ready: []*corev1.Node{ - newNodeWithState(machineConfigV0, machineConfigV0, "Done"), - }, - unavailable: []*corev1.Node{ - newNodeWithState(machineConfigV0, machineConfigV0, "Degraded"), - newNodeWithState(machineConfigV0, machineConfigV0, "Unreconcilable"), - newNodeWithState(machineConfigV0, machineConfigV1, "Working"), - newNodeWithState(machineConfigV0, machineConfigV1, "Rebooting"), - }, - degraded: []*corev1.Node{ - newNodeWithState(machineConfigV0, machineConfigV0, "Degraded"), - newNodeWithState(machineConfigV0, machineConfigV0, "Unreconcilable"), - }, - }, { - name: "Layered nodes with mixed states", - nodes: []*corev1.Node{ - newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Degraded"), - newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Done"), - newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Unreconcilable"), - newLayeredNodeWithState(machineConfigV0, machineConfigV1, imageV1, imageV1, "Working"), - newLayeredNodeWithState(machineConfigV0, machineConfigV1, imageV1, imageV1, "Rebooting"), - }, - currentConfig: machineConfigV0, - currentImage: imageV1, - layered: true, - mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), - mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV0).MachineOSBuild(), - updated: []*corev1.Node{ - newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Done"), - }, - ready: []*corev1.Node{ - newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Done"), - }, - unavailable: []*corev1.Node{ - newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Degraded"), - newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Unreconcilable"), - newLayeredNodeWithState(machineConfigV0, machineConfigV1, imageV1, imageV1, "Working"), - newLayeredNodeWithState(machineConfigV0, machineConfigV1, imageV1, imageV1, "Rebooting"), - }, - degraded: []*corev1.Node{ - newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Degraded"), - newLayeredNodeWithState(machineConfigV0, machineConfigV0, imageV1, imageV1, "Unreconcilable"), - }, - }, { - name: "0 layered nodes updated, 2 degraded", - nodes: []*corev1.Node{ - newLayeredNodeWithState(machineConfigV1, machineConfigV0, imageV1, imageV1, "Degraded"), - newLayeredNodeWithState(machineConfigV1, machineConfigV0, imageV1, imageV1, "Unreconcilable"), - newLayeredNode(machineConfigV0, machineConfigV0, imageV1, imageV1), - newLayeredNode(machineConfigV0, machineConfigV0, imageV1, imageV1), - }, - currentConfig: machineConfigV1, - currentImage: imageV1, - layered: true, - mosc: helpers.NewMachineOSConfigBuilder("mosc-1").WithCurrentImagePullspec(imageV1).WithMachineConfigPool("pool-1").MachineOSConfig(), - mosb: helpers.NewMachineOSBuildBuilder("mosb-1").WithDesiredConfig(machineConfigV0).MachineOSBuild(), - degraded: []*corev1.Node{ - newLayeredNodeWithState(machineConfigV1, machineConfigV0, imageV1, imageV1, "Degraded"), - newLayeredNodeWithState(machineConfigV1, machineConfigV0, imageV1, imageV1, "Unreconcilable"), - }, - }, } for _, test := range tests { @@ -1940,37 +1953,37 @@ func TestDetectRuncInMachineConfig(t *testing.T) { { name: "runc in single drop-in", mc: helpers.NewMachineConfig("test-runc", nil, "", []ign3types.File{ - helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("runc"), 0644), + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("runc"), 0o644), }), expectedMC: "test-runc", }, { name: "crun in single drop-in", mc: helpers.NewMachineConfig("test", nil, "", []ign3types.File{ - helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("crun"), 0644), + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("crun"), 0o644), }), expectedMC: "", }, { name: "runc overridden by crun in later drop-in", mc: helpers.NewMachineConfig("test", nil, "", []ign3types.File{ - helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("runc"), 0644), - helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/01-ctrcfg", makeCRIODropIn("crun"), 0644), + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("runc"), 0o644), + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/01-ctrcfg", makeCRIODropIn("crun"), 0o644), }), expectedMC: "", }, { name: "crun overridden by runc in later drop-in", mc: helpers.NewMachineConfig("test-runc-override", nil, "", []ign3types.File{ - helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("crun"), 0644), - helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/01-ctrcfg", makeCRIODropIn("runc"), 0644), + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("crun"), 0o644), + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/01-ctrcfg", makeCRIODropIn("runc"), 0o644), }), expectedMC: "test-runc-override", }, { name: "non-CRI-O files are ignored", mc: helpers.NewMachineConfig("test", nil, "", []ign3types.File{ - helpers.CreateEncodedIgn3File("/etc/other/config", makeCRIODropIn("runc"), 0644), + helpers.CreateEncodedIgn3File("/etc/other/config", makeCRIODropIn("runc"), 0o644), }), expectedMC: "", }, @@ -1984,7 +1997,7 @@ func TestDetectRuncInMachineConfig(t *testing.T) { { name: "runc in gzip-compressed drop-in", mc: func() *mcfgv1.MachineConfig { - gzFile, err := helpers.CreateGzippedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("runc"), 0644) + gzFile, err := helpers.CreateGzippedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("runc"), 0o644) if err != nil { t.Fatalf("failed to create gzipped file: %v", err) } @@ -2072,3 +2085,202 @@ func TestGetAllValidPackageSetsForExtension(t *testing.T) { } } +func TestGetCryptoPolicyFromTLSProfile(t *testing.T) { + tests := []struct { + name string + profile *configv1.TLSSecurityProfile + expectedPolicy string + expectedSubMod string + }{ + { + name: "nil profile defaults to DEFAULT", + profile: nil, + expectedPolicy: "DEFAULT", + expectedSubMod: "", + }, + { + name: "Intermediate maps to DEFAULT", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileIntermediateType, + Intermediate: &configv1.IntermediateTLSProfile{}, + }, + expectedPolicy: "DEFAULT", + expectedSubMod: "", + }, + { + name: "Modern maps to DEFAULT:OPENSHIFT with decomposed sub-policy", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileModernType, + Modern: &configv1.ModernTLSProfile{}, + }, + expectedPolicy: "DEFAULT:OPENSHIFT", + expectedSubMod: "cipher@TLS = AES-128-GCM AES-256-GCM CHACHA20-POLY1305\n" + + "mac@TLS = AEAD\n" + + "protocol@TLS = -TLS1.0 -TLS1.1 -TLS1.2 -DTLS1.0 -DTLS1.2\n" + + "group@TLS = MLKEM768-X25519 X25519 SECP256R1 SECP384R1", + }, + { + name: "Old maps to LEGACY:OPENSHIFT with decomposed sub-policy", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileOldType, + Old: &configv1.OldTLSProfile{}, + }, + expectedPolicy: "LEGACY:OPENSHIFT", + expectedSubMod: "cipher@TLS = 3DES-CBC AES-128-CBC AES-128-GCM AES-256-CBC AES-256-GCM CHACHA20-POLY1305\n" + + "mac@TLS = AEAD HMAC-SHA1 HMAC-SHA2-256 HMAC-SHA2-384\n" + + "protocol@TLS = -TLS1.0 -TLS1.1 -DTLS1.0\n" + + "group@TLS = MLKEM768-X25519 X25519 SECP256R1 SECP384R1", + }, + { + name: "Custom with Intermediate-equivalent ciphers and TLS 1.2", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: &configv1.CustomTLSProfile{ + TLSProfileSpec: configv1.TLSProfileSpec{ + Ciphers: []string{ + "TLS_AES_128_GCM_SHA256", + "TLS_AES_256_GCM_SHA384", + "TLS_CHACHA20_POLY1305_SHA256", + "ECDHE-ECDSA-AES128-GCM-SHA256", + "ECDHE-RSA-AES128-GCM-SHA256", + "ECDHE-ECDSA-AES256-GCM-SHA384", + "ECDHE-RSA-AES256-GCM-SHA384", + "ECDHE-ECDSA-CHACHA20-POLY1305", + "ECDHE-RSA-CHACHA20-POLY1305", + }, + MinTLSVersion: configv1.VersionTLS12, + }, + }, + }, + expectedPolicy: "DEFAULT:OPENSHIFT", + expectedSubMod: "cipher@TLS = AES-128-GCM AES-256-GCM CHACHA20-POLY1305\nmac@TLS = AEAD\nprotocol@TLS = -TLS1.0 -TLS1.1 -DTLS1.0", + }, + { + name: "Custom with TLS 1.3 only ciphers", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: &configv1.CustomTLSProfile{ + TLSProfileSpec: configv1.TLSProfileSpec{ + Ciphers: []string{ + "TLS_AES_256_GCM_SHA384", + "TLS_CHACHA20_POLY1305_SHA256", + }, + MinTLSVersion: configv1.VersionTLS13, + }, + }, + }, + expectedPolicy: "DEFAULT:OPENSHIFT", + expectedSubMod: "cipher@TLS = AES-256-GCM CHACHA20-POLY1305\nmac@TLS = AEAD\nprotocol@TLS = -TLS1.0 -TLS1.1 -TLS1.2 -DTLS1.0 -DTLS1.2", + }, + { + name: "Custom without groups (non-TechPreview: API strips groups field)", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: &configv1.CustomTLSProfile{ + TLSProfileSpec: configv1.TLSProfileSpec{ + Ciphers: []string{ + "TLS_AES_256_GCM_SHA384", + "TLS_AES_128_GCM_SHA256", + }, + MinTLSVersion: configv1.VersionTLS13, + }, + }, + }, + expectedPolicy: "DEFAULT:OPENSHIFT", + expectedSubMod: "cipher@TLS = AES-128-GCM AES-256-GCM\nmac@TLS = AEAD\nprotocol@TLS = -TLS1.0 -TLS1.1 -TLS1.2 -DTLS1.0 -DTLS1.2", + }, + { + name: "Custom with groups (TechPreview: TLSGroupPreferences gate enabled)", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: &configv1.CustomTLSProfile{ + TLSProfileSpec: configv1.TLSProfileSpec{ + Ciphers: []string{ + "TLS_AES_256_GCM_SHA384", + }, + MinTLSVersion: configv1.VersionTLS13, + Groups: []configv1.TLSGroup{ + configv1.TLSGroupX25519MLKEM768, + configv1.TLSGroupX25519, + configv1.TLSGroupSecP256r1, + }, + }, + }, + }, + expectedPolicy: "DEFAULT:OPENSHIFT", + expectedSubMod: "cipher@TLS = AES-256-GCM\nmac@TLS = AEAD\nprotocol@TLS = -TLS1.0 -TLS1.1 -TLS1.2 -DTLS1.0 -DTLS1.2\ngroup@TLS = MLKEM768-X25519 X25519 SECP256R1", + }, + { + name: "Custom with all groups (TechPreview: full group set)", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: &configv1.CustomTLSProfile{ + TLSProfileSpec: configv1.TLSProfileSpec{ + Ciphers: []string{ + "TLS_AES_256_GCM_SHA384", + }, + MinTLSVersion: configv1.VersionTLS13, + Groups: []configv1.TLSGroup{ + configv1.TLSGroupX25519MLKEM768, + configv1.TLSGroupSecP256r1MLKEM768, + configv1.TLSGroupSecP384r1MLKEM1024, + configv1.TLSGroupX25519, + configv1.TLSGroupSecP256r1, + configv1.TLSGroupSecP384r1, + configv1.TLSGroupSecP521r1, + }, + }, + }, + }, + expectedPolicy: "DEFAULT:OPENSHIFT", + expectedSubMod: "cipher@TLS = AES-256-GCM\nmac@TLS = AEAD\nprotocol@TLS = -TLS1.0 -TLS1.1 -TLS1.2 -DTLS1.0 -DTLS1.2\ngroup@TLS = MLKEM768-X25519 P256-MLKEM768 P384-MLKEM1024 X25519 SECP256R1 SECP384R1 SECP521R1", + }, + { + name: "Custom with CBC ciphers includes non-AEAD MACs", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: &configv1.CustomTLSProfile{ + TLSProfileSpec: configv1.TLSProfileSpec{ + Ciphers: []string{ + "ECDHE-RSA-AES256-GCM-SHA384", + "ECDHE-RSA-AES128-SHA256", + }, + MinTLSVersion: configv1.VersionTLS12, + }, + }, + }, + expectedPolicy: "DEFAULT:OPENSHIFT", + expectedSubMod: "cipher@TLS = AES-128-CBC AES-256-GCM\nmac@TLS = AEAD HMAC-SHA2-256\nprotocol@TLS = -TLS1.0 -TLS1.1 -DTLS1.0", + }, + { + name: "Custom with nil spec falls back to DEFAULT", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: nil, + }, + expectedPolicy: "DEFAULT", + expectedSubMod: "", + }, + { + name: "Custom with no ciphers still generates protocol directive", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: &configv1.CustomTLSProfile{ + TLSProfileSpec: configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS12, + }, + }, + }, + expectedPolicy: "DEFAULT:OPENSHIFT", + expectedSubMod: "protocol@TLS = -TLS1.0 -TLS1.1 -DTLS1.0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + policy, subMod := GetCryptoPolicyFromTLSProfile(tt.profile) + assert.Equal(t, tt.expectedPolicy, policy) + assert.Equal(t, tt.expectedSubMod, subMod) + }) + } +} diff --git a/pkg/controller/template/kubelet_config_dir_test.go b/pkg/controller/template/kubelet_config_dir_test.go index 722a04e505..34a5f8dced 100644 --- a/pkg/controller/template/kubelet_config_dir_test.go +++ b/pkg/controller/template/kubelet_config_dir_test.go @@ -46,7 +46,7 @@ func TestKubeletConfigDirParameter(t *testing.T) { controllerConfig, err := controllerConfigFromFile(tc.controllerConfig) require.NoError(t, err, "Failed to load controller config for %s", tc.name) - cfgs, err := generateTemplateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, nil}, templateDir) + cfgs, err := generateTemplateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, "", "", nil}, templateDir) require.NoError(t, err, "Failed to generate machine configs for %s", tc.name) for _, cfg := range cfgs { @@ -73,7 +73,7 @@ func TestKubeletConfigDirParameterSpecific(t *testing.T) { controllerConfig, err := controllerConfigFromFile("./test_data/controller_config_aws.yaml") require.NoError(t, err, "Failed to load AWS controller config") - cfgs, err := generateTemplateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, nil}, templateDir) + cfgs, err := generateTemplateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, "", "", nil}, templateDir) require.NoError(t, err, "Failed to generate machine configs") var kubeletUnit *string diff --git a/pkg/controller/template/render.go b/pkg/controller/template/render.go index 39492770f9..bb8ac5f3cb 100644 --- a/pkg/controller/template/render.go +++ b/pkg/controller/template/render.go @@ -43,9 +43,11 @@ const ( // RenderConfig is wrapper around ControllerConfigSpec. type RenderConfig struct { *mcfgv1.ControllerConfigSpec - PullSecret string - TLSMinVersion string - TLSCipherSuites []string + PullSecret string + TLSMinVersion string + TLSCipherSuites []string + CryptoPolicy string + CryptoPolicySubMod string // no need to set this, will be automatically configured Constants map[string]string diff --git a/pkg/controller/template/render_test.go b/pkg/controller/template/render_test.go index 10e6ab739b..fdd5ae571d 100644 --- a/pkg/controller/template/render_test.go +++ b/pkg/controller/template/render_test.go @@ -82,7 +82,7 @@ func TestCloudProvider(t *testing.T) { }, } - got, err := renderTemplate(RenderConfig{&config.Spec, `{"dummy":"dummy"}`, "dummy", nil, nil}, name, dummyTemplate) + got, err := renderTemplate(RenderConfig{&config.Spec, `{"dummy":"dummy"}`, "dummy", nil, "", "", nil}, name, dummyTemplate) if err != nil { t.Fatalf("expected nil error %v", err) } @@ -143,7 +143,7 @@ func TestCredentialProviderConfigFlag(t *testing.T) { }, } - got, err := renderTemplate(RenderConfig{&config.Spec, `{"dummy":"dummy"}`, "dummy", nil, nil}, name, dummyTemplate) + got, err := renderTemplate(RenderConfig{&config.Spec, `{"dummy":"dummy"}`, "dummy", nil, "", "", nil}, name, dummyTemplate) if err != nil { t.Fatalf("expected nil error %v", err) } @@ -162,23 +162,24 @@ func TestSkipMissing(t *testing.T) { key string err bool res string - }{{ - key: "", - err: true, - res: "", - }, { - key: "2two", - err: true, - res: "", - }, { - key: "test index", - err: true, - res: "", - }, { - key: "index", - err: false, - res: "{{.index}}", - }, + }{ + { + key: "", + err: true, + res: "", + }, { + key: "2two", + err: true, + res: "", + }, { + key: "test index", + err: true, + res: "", + }, { + key: "index", + err: false, + res: "{{.index}}", + }, } for idx, c := range cases { @@ -199,26 +200,24 @@ func TestSkipMissing(t *testing.T) { const templateDir = "../../../templates" -var ( - configs = map[string]string{ - "aws": "./test_data/controller_config_aws.yaml", - "baremetal": "./test_data/controller_config_baremetal.yaml", - "baremetal-arbiter": "./test_data/controller_config_baremetal_arbiter.yaml", - "gcp": "./test_data/controller_config_gcp.yaml", - "openstack": "./test_data/controller_config_openstack.yaml", - "libvirt": "./test_data/controller_config_libvirt.yaml", - "mtu-migration": "./test_data/controller_config_mtu_migration.yaml", - "none": "./test_data/controller_config_none.yaml", - "external": "./test_data/controller_config_external.yaml", - "vsphere": "./test_data/controller_config_vsphere.yaml", - "kubevirt": "./test_data/controller_config_kubevirt.yaml", - "powervs": "./test_data/controller_config_powervs.yaml", - "nutanix": "./test_data/controller_config_nutanix.yaml", - "gcp-custom-dns": "./test_data/controller_config_gcp_custom_dns.yaml", - "gcp-default-dns": "./test_data/controller_config_gcp_default_dns.yaml", - "baremetal-tnf": "./test_data/controller_config_baremetal_tnf.yaml", - } -) +var configs = map[string]string{ + "aws": "./test_data/controller_config_aws.yaml", + "baremetal": "./test_data/controller_config_baremetal.yaml", + "baremetal-arbiter": "./test_data/controller_config_baremetal_arbiter.yaml", + "gcp": "./test_data/controller_config_gcp.yaml", + "openstack": "./test_data/controller_config_openstack.yaml", + "libvirt": "./test_data/controller_config_libvirt.yaml", + "mtu-migration": "./test_data/controller_config_mtu_migration.yaml", + "none": "./test_data/controller_config_none.yaml", + "external": "./test_data/controller_config_external.yaml", + "vsphere": "./test_data/controller_config_vsphere.yaml", + "kubevirt": "./test_data/controller_config_kubevirt.yaml", + "powervs": "./test_data/controller_config_powervs.yaml", + "nutanix": "./test_data/controller_config_nutanix.yaml", + "gcp-custom-dns": "./test_data/controller_config_gcp_custom_dns.yaml", + "gcp-default-dns": "./test_data/controller_config_gcp_default_dns.yaml", + "baremetal-tnf": "./test_data/controller_config_baremetal_tnf.yaml", +} func TestInvalidPlatform(t *testing.T) { controllerConfig, err := controllerConfigFromFile(configs["aws"]) @@ -238,14 +237,14 @@ func TestInvalidPlatform(t *testing.T) { // we must treat unrecognized constants as "none" controllerConfig.Spec.Infra.Status.PlatformStatus.Type = "_bad_" - _, err = generateTemplateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, nil}, templateDir) + _, err = generateTemplateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, "", "", nil}, templateDir) if err != nil { t.Errorf("expect nil error, got: %v", err) } // explicitly blocked controllerConfig.Spec.Infra.Status.PlatformStatus.Type = "_base" - _, err = generateTemplateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, nil}, templateDir) + _, err = generateTemplateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, "", "", nil}, templateDir) expectErr(err, "failed to create MachineConfig for role master: platform _base unsupported") } @@ -256,7 +255,7 @@ func TestGenerateMachineConfigs(t *testing.T) { t.Fatalf("failed to get controllerconfig config: %v", err) } - cfgs, err := generateTemplateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, nil}, templateDir) + cfgs, err := generateTemplateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, "", "", nil}, templateDir) if err != nil { t.Fatalf("failed to generate machine configs: %v", err) } @@ -378,7 +377,7 @@ func TestKubeletGracefulShutdownTNF(t *testing.T) { } cfgs, err := generateTemplateMachineConfigs( - &RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, nil}, + &RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, "", "", nil}, templateDir, ) if err != nil { @@ -457,7 +456,6 @@ func TestGetPaths(t *testing.T) { res: []string{strings.ToLower(string(configv1.GCPPlatformType))}, topology: configv1.HighlyAvailableTopologyMode, }, { - platform: configv1.NonePlatformType, res: []string{strings.ToLower(string(configv1.NonePlatformType)), sno}, topology: configv1.SingleReplicaTopologyMode, @@ -512,7 +510,7 @@ func TestGetPaths(t *testing.T) { } c.res = append(c.res, platformBase) - got := getPaths(&RenderConfig{&config.Spec, `{"dummy":"dummy"}`, "dummy", nil, nil}, config.Spec.Platform) + got := getPaths(&RenderConfig{&config.Spec, `{"dummy":"dummy"}`, "dummy", nil, "", "", nil}, config.Spec.Platform) if reflect.DeepEqual(got, c.res) { t.Fatalf("mismatch got: %s want: %s", got, c.res) } @@ -555,7 +553,6 @@ func findIgnUnit(units []ign3types.Unit, name string, t *testing.T) bool { } func verifyIgn(actual [][]byte, dir string, t *testing.T) { - expected := make(map[string][]byte) if err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { if err != nil { @@ -596,3 +593,73 @@ func verifyIgn(actual [][]byte, dir string, t *testing.T) { t.Errorf("can't find expected file:\n%v", key) } } + +func TestCryptoPolicyTemplateRendering(t *testing.T) { + configTemplate := []byte(`{{ .CryptoPolicy }}`) + subModTemplate := []byte(`{{- if .CryptoPolicySubMod }}{{ .CryptoPolicySubMod }}{{- end }}`) + + tests := []struct { + name string + cryptoPolicy string + cryptoSubMod string + template []byte + expectedOutput string + }{ + { + name: "DEFAULT policy renders policy name", + cryptoPolicy: "DEFAULT", + cryptoSubMod: "", + template: configTemplate, + expectedOutput: "DEFAULT", + }, + { + name: "DEFAULT:OPENSHIFT policy renders policy name", + cryptoPolicy: "DEFAULT:OPENSHIFT", + cryptoSubMod: "protocol@TLS = -TLS1.0 -TLS1.1 -TLS1.2 -DTLS1.0 -DTLS1.2", + template: configTemplate, + expectedOutput: "DEFAULT:OPENSHIFT", + }, + { + name: "sub-policy module renders when set", + cryptoPolicy: "DEFAULT:OPENSHIFT", + cryptoSubMod: "protocol@TLS = -TLS1.0 -TLS1.1 -TLS1.2 -DTLS1.0 -DTLS1.2", + template: subModTemplate, + expectedOutput: "protocol@TLS = -TLS1.0 -TLS1.1 -TLS1.2 -DTLS1.0 -DTLS1.2", + }, + { + name: "sub-policy module omitted when empty", + cryptoPolicy: "DEFAULT", + cryptoSubMod: "", + template: subModTemplate, + expectedOutput: "", + }, + { + name: "multi-line sub-policy module renders for Custom profile", + cryptoPolicy: "DEFAULT:OPENSHIFT", + cryptoSubMod: "cipher@TLS = AES-128-GCM AES-256-GCM CHACHA20-POLY1305\nmac@TLS = AEAD\nprotocol@TLS = -TLS1.0 -TLS1.1 -DTLS1.0", + template: subModTemplate, + expectedOutput: "cipher@TLS = AES-128-GCM AES-256-GCM CHACHA20-POLY1305\nmac@TLS = AEAD\nprotocol@TLS = -TLS1.0 -TLS1.1 -DTLS1.0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config := &mcfgv1.ControllerConfig{ + Spec: mcfgv1.ControllerConfigSpec{}, + } + rc := RenderConfig{ + ControllerConfigSpec: &config.Spec, + PullSecret: `{"dummy":"dummy"}`, + CryptoPolicy: tt.cryptoPolicy, + CryptoPolicySubMod: tt.cryptoSubMod, + } + got, err := renderTemplate(rc, tt.name, tt.template) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(got) != tt.expectedOutput { + t.Fatalf("mismatch got: %q want: %q", string(got), tt.expectedOutput) + } + }) + } +} diff --git a/pkg/controller/template/template_controller.go b/pkg/controller/template/template_controller.go index bd7d6f8bc0..90462c49e4 100644 --- a/pkg/controller/template/template_controller.go +++ b/pkg/controller/template/template_controller.go @@ -240,14 +240,12 @@ func (ctrl *Controller) addAPIServer(obj interface{}) { } func (ctrl *Controller) updateAPIServer(old, cur interface{}) { - oldAPIServer := old.(*configv1.APIServer) newAPIServer := cur.(*configv1.APIServer) if !reflect.DeepEqual(oldAPIServer.Spec, newAPIServer.Spec) { klog.V(4).Infof("Updating APIServer: %s", newAPIServer.Name) ctrl.filterAPIServer(newAPIServer) } - } func (ctrl *Controller) deleteAPIServer(obj interface{}) { @@ -668,11 +666,20 @@ func getMachineConfigsForControllerConfig(templatesDir string, config *mcfgv1.Co return nil, fmt.Errorf("couldn't compact pullsecret %q: %w", string(clusterPullSecretRaw), err) } tlsMinVersion, tlsCipherSuites := ctrlcommon.GetSecurityProfileCiphersFromAPIServer(apiServer) + + var tlsProfile *configv1.TLSSecurityProfile + if apiServer != nil { + tlsProfile = apiServer.Spec.TLSSecurityProfile + } + cryptoPolicy, cryptoPolicySubMod := ctrlcommon.GetCryptoPolicyFromTLSProfile(tlsProfile) + rc := &RenderConfig{ ControllerConfigSpec: &config.Spec, - PullSecret: string(buf.Bytes()), + PullSecret: buf.String(), TLSMinVersion: tlsMinVersion, TLSCipherSuites: tlsCipherSuites, + CryptoPolicy: cryptoPolicy, + CryptoPolicySubMod: cryptoPolicySubMod, } mcs, err := generateTemplateMachineConfigs(rc, templatesDir) if err != nil { diff --git a/pkg/daemon/file_writers.go b/pkg/daemon/file_writers.go index 8df8f44089..b9f7618c13 100644 --- a/pkg/daemon/file_writers.go +++ b/pkg/daemon/file_writers.go @@ -225,12 +225,18 @@ func writeDropins(u ign3types.Unit, systemdRoot string, isCoreOSVariant bool) er // writeFiles writes the given files to disk. // it doesn't fetch remote files and expects a flattened config file. func writeFiles(files []ign3types.File, skipCertificateWrite bool) error { + skipCryptoPolicyFiles := getCryptoPolicyFilesToSkip() + for _, file := range files { if skipCertificateWrite && file.Path == caBundleFilePath { // TODO remove this special case once we have a better way to do this klog.V(4).Infof("Skipping file %s during writeFiles", caBundleFilePath) continue } + if skipCryptoPolicyFiles.Has(file.Path) { + klog.Infof("Skipping file %s during writeFiles: FIPS manages crypto-policy on this node", file.Path) + continue + } klog.Infof("Writing file %q", file.Path) // We don't support appends in the file section, so instead of waiting to fail validation, diff --git a/pkg/daemon/on_disk_validation.go b/pkg/daemon/on_disk_validation.go index 459d7acd10..9226a830fc 100644 --- a/pkg/daemon/on_disk_validation.go +++ b/pkg/daemon/on_disk_validation.go @@ -167,16 +167,42 @@ func checkV2Units(units []ign2types.Unit, systemdPath string) error { return nil } +// getCryptoPolicyFilesToSkip returns crypto-policy file paths that should be +// skipped on FIPS nodes. On FIPS, the rhcos-fips dracut module writes "FIPS" +// to /etc/crypto-policies/config during early boot. If Ignition later +// overwrites it (e.g. with "DEFAULT"), fips-crypto-policy-overlay detects the +// mismatch and bind-mounts the FIPS policy over the file. Either way, the +// on-disk content will read as "FIPS" regardless of what the MachineConfig +// wrote — writing would fail (EBUSY if bind-mounted) or produce false +// content mismatches during validation. +func getCryptoPolicyFilesToSkip() sets.Set[string] { + result := sets.New[string]() + if err := processFips(func(nodeFIPS bool) error { + if nodeFIPS { + result.Insert( + "/etc/crypto-policies/config", + "/etc/crypto-policies/policies/modules/OPENSHIFT.pmod", + ) + } + return nil + }); err != nil { + klog.Warningf("Could not determine FIPS status for crypto-policy file skip: %v", err) + } + return result +} + // To transition certain files from being under MachineConfig management to // certificate_Writer, we must ignore these files at validation time since // there is the possibility that certificate_writer may write different // contents to the file during the transition. func getFilesToIgnore() sets.Set[string] { - return sets.New[string]( + ignoredFiles := sets.New[string]( caBundleFilePath, cloudCABundleFilePath, internalRegistryAuthFile, ) + ignoredFiles = ignoredFiles.Union(getCryptoPolicyFilesToSkip()) + return ignoredFiles } // checkV3Files validates the contents of all the files in the target config. diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 8f107bbec8..c2999b1776 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -748,7 +748,6 @@ func calculatePostConfigChangeAction(diff *machineConfigDiff, diffFileSet []stri // calculatePostConfigChangeNodeDisruptionAction takes action based on the cluster's Node disruption policies. func (dn *Daemon) calculatePostConfigChangeNodeDisruptionAction(diff *machineConfigDiff, diffFileSet, diffUnitSet []string) ([]opv1.NodeDisruptionPolicyStatusAction, error) { - var mcop *opv1.MachineConfiguration var pollErr error // Wait for mcop.Status.NodeDisruptionPolicyStatus to populate, otherwise error out. This shouldn't take very long @@ -819,7 +818,6 @@ func (dn *Daemon) calculatePostConfigChangeNodeDisruptionAction(diff *machineCon } return nodeDisruptionActions, nil - } // Finalizes the revert process by enabling a special systemd unit prior to @@ -1312,6 +1310,11 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi return err } + // update crypto policy + if err := applyCryptoPolicy(diffFileSet); err != nil { + return err + } + // At this point, we write the now expected to be "current" config to /etc. // When we reboot, we'll find this file and validate that we're in this state, // and that completes an update. @@ -1775,7 +1778,6 @@ func (dn *Daemon) getCurrentlyInstalledPackages() (sets.Set[string], error) { // generateExtensionsArgs generates extension arguments for rpm-ostree, based on the target config // and currently installed extension packages. func generateExtensionsArgs(installedSet sets.Set[string], newConfig *mcfgv1.MachineConfig) []string { - // Get packages that should be installed based on new config supportedExtensions := ctrlcommon.SupportedExtensions() requiredSet := sets.New[string]() @@ -2511,7 +2513,6 @@ func (dn *Daemon) listSystemdUnits() (result map[string]systemddbus.UnitFile, er result[unitName] = unitFile } return result, nil - } // writeFiles writes the given files to disk. @@ -2607,7 +2608,6 @@ func getUserPasswordHash(user string) (string, error) { return shadowSlice[1], nil } return "", nil - } // SetPasswordHash updates the password for each user in newUsers, skipping @@ -2671,6 +2671,49 @@ func (dn *Daemon) updateKubeConfigPermission() error { return nil } +// applyCryptoPolicy runs update-crypto-policies when the rendered MachineConfig +// changes /etc/crypto-policies/config or any .pmod under policies/modules/. +// On FIPS nodes it refuses to apply a non-FIPS policy. +func applyCryptoPolicy(diffFileSet []string) error { + needsUpdate := false + for _, path := range diffFileSet { + if path == "/etc/crypto-policies/config" || strings.HasPrefix(path, "/etc/crypto-policies/policies/modules/") { + needsUpdate = true + break + } + } + if !needsUpdate { + return nil + } + + if err := processFips(func(nodeFIPS bool) error { + if !nodeFIPS { + return nil + } + desiredBytes, err := os.ReadFile("/etc/crypto-policies/config") + if err != nil { + return fmt.Errorf("failed to read /etc/crypto-policies/config: %w", err) + } + desired := strings.TrimSpace(string(desiredBytes)) + if strings.HasPrefix(desired, "FIPS") { + return nil + } + klog.Warningf("Skipping crypto-policy update: FIPS mode is enabled but desired policy is %q; FIPS crypto-policy takes precedence", desired) + return errSkipCryptoPolicy + }); err != nil { + if errors.Is(err, errSkipCryptoPolicy) { + return nil + } + return err + } + + klog.Infof("Applying crypto-policy via update-crypto-policies") + if err := runCmdSync("update-crypto-policies"); err != nil { + return fmt.Errorf("failed to apply crypto-policy: %w", err) + } + return nil +} + // Determines if we should use the new SSH key path // (/home/core/.ssh/authorized_keys.d/ignition) or the old SSH key path // (/home/core/.ssh/authorized_keys) @@ -3069,6 +3112,8 @@ func runCmdSync(cmdName string, args ...string) error { return nil } +var errSkipCryptoPolicy = errors.New("skip crypto-policy update") + var ( podmanSigstoreSupported sync.Once podmanSigstoreSupportedValue bool @@ -3274,9 +3319,8 @@ func (dn *CoreOSDaemon) applyLayeredOSChanges(mcDiff machineConfigDiff, oldConfi // repo isn't there rpm-ostree will fail if there are layered packages. // See https://redhat.atlassian.net/browse/OCPBUGS-2269 haveExtensions := len(oldConfig.Spec.Extensions) != 0 || len(newConfig.Spec.Extensions) != 0 - haveKernelType := - helpers.CanonicalizeKernelType(oldConfig.Spec.KernelType) != ctrlcommon.KernelTypeDefault || - helpers.CanonicalizeKernelType(newConfig.Spec.KernelType) != ctrlcommon.KernelTypeDefault + haveKernelType := helpers.CanonicalizeKernelType(oldConfig.Spec.KernelType) != ctrlcommon.KernelTypeDefault || + helpers.CanonicalizeKernelType(newConfig.Spec.KernelType) != ctrlcommon.KernelTypeDefault var osExtensionsContentDir string var err error diff --git a/pkg/daemon/update_test.go b/pkg/daemon/update_test.go index 0490e438d5..5f3dd5896a 100644 --- a/pkg/daemon/update_test.go +++ b/pkg/daemon/update_test.go @@ -48,7 +48,7 @@ func setupTempDirWithEtc(t *testing.T) (string, func()) { // Stub out a test directory structure -- we need to create /etc so createOrigFile can use it etcDir := filepath.Join(testDir, "etc") - err := os.MkdirAll(etcDir, 0755) + err := os.MkdirAll(etcDir, 0o755) require.Nil(t, err) oldOrigParentDirPath := origParentDirPath @@ -241,9 +241,11 @@ func TestMachineConfigDiff(t *testing.T) { } func newTestIgnitionFile(i uint) ign3types.File { - mode := 0644 - return ign3types.File{Node: ign3types.Node{Path: fmt.Sprintf("/etc/config%d", i)}, - FileEmbedded1: ign3types.FileEmbedded1{Contents: ign3types.Resource{Source: helpers.StrToPtr(fmt.Sprintf("data:,config%d", i))}, Mode: &mode}} + mode := 0o644 + return ign3types.File{ + Node: ign3types.Node{Path: fmt.Sprintf("/etc/config%d", i)}, + FileEmbedded1: ign3types.FileEmbedded1{Contents: ign3types.Resource{Source: helpers.StrToPtr(fmt.Sprintf("data:,config%d", i))}, Mode: &mode}, + } } func newMachineConfigFromFiles(files []ign3types.File) *mcfgv1.MachineConfig { @@ -311,20 +313,26 @@ func TestKernelAguments(t *testing.T) { { oldKargs: []string{"foo", "bar=1 hello=world", "baz"}, newKargs: []string{"foo", "bar=1", "hello=world"}, - out: []string{"--delete-if-present=foo", "--delete-if-present=bar=1", "--delete-if-present=hello=world", "--delete-if-present=baz", - "--append=foo", "--append=bar=1", "--append=hello=world"}, + out: []string{ + "--delete-if-present=foo", "--delete-if-present=bar=1", "--delete-if-present=hello=world", "--delete-if-present=baz", + "--append=foo", "--append=bar=1", "--append=hello=world", + }, }, { oldKargs: []string{" baz=test bar=\"hello world\""}, newKargs: []string{" baz=test bar=\"hello world\"", "foo"}, - out: []string{"--delete-if-present=baz=test", "--delete-if-present=bar=\"hello world\"", - "--append=baz=test", "--append=bar=\"hello world\"", "--append=foo"}, + out: []string{ + "--delete-if-present=baz=test", "--delete-if-present=bar=\"hello world\"", + "--append=baz=test", "--append=bar=\"hello world\"", "--append=foo", + }, }, { oldKargs: []string{"hugepagesz=1G hugepages=4", "hugepagesz=2M hugepages=4"}, newKargs: []string{"hugepagesz=1G hugepages=4", "hugepagesz=2M hugepages=6"}, - out: []string{"--delete-if-present=hugepagesz=1G", "--delete-if-present=hugepages=4", "--delete-if-present=hugepagesz=2M", "--delete-if-present=hugepages=4", - "--append=hugepagesz=1G", "--append=hugepages=4", "--append=hugepagesz=2M", "--append=hugepages=6"}, + out: []string{ + "--delete-if-present=hugepagesz=1G", "--delete-if-present=hugepages=4", "--delete-if-present=hugepagesz=2M", "--delete-if-present=hugepages=4", + "--append=hugepagesz=1G", "--append=hugepages=4", "--append=hugepagesz=2M", "--append=hugepages=6", + }, }, } @@ -435,7 +443,6 @@ func TestUpdateSSHKeys(t *testing.T) { err := d.updateSSHKeys(newIgnCfg.Passwd.Users, oldIgnConfig.Passwd.Users) if err != nil { t.Errorf("Expected no error. Got %s.", err) - } // if Users is empty, nothing should happen and no error should ever be generated @@ -640,7 +647,8 @@ func TestCalculatePostConfigChangeAction(t *testing.T) { // test that updating openshift-config-user-ca-bundle.crt is crio restart oldConfig: helpers.NewMachineConfig("00-test", nil, "dummy://", []ign3types.File{files["restart-crio1"]}), newConfig: helpers.NewMachineConfig("01-test", nil, "dummy://", []ign3types.File{files["restart-crio2"]}), - expectedAction: []string{postConfigChangeActionRestartCrio}}, + expectedAction: []string{postConfigChangeActionRestartCrio}, + }, { // test that updating openshift-config-user-ca-bundle.crt is crio restart and that it overrides a following crio reload oldConfig: helpers.NewMachineConfig("00-test", nil, "dummy://", []ign3types.File{files["restart-crio1"]}), @@ -699,7 +707,7 @@ func TestOriginalFileBackupRestore(t *testing.T) { // Write a file in the /tmp dir to test whether orig files are selectively preserved controlFile := filepath.Join(testDir, "control-file") - err := os.WriteFile(controlFile, []byte("control file contents"), 0755) + err := os.WriteFile(controlFile, []byte("control file contents"), 0o755) assert.Nil(t, err) // Back up the tmp file @@ -716,13 +724,13 @@ func TestOriginalFileBackupRestore(t *testing.T) { } func TestFindClosestFilePolicyPathMatch(t *testing.T) { - policyActions := map[string][]opv1.NodeDisruptionPolicyStatusAction{ "Empty": {}, "None": {{Type: opv1.NoneStatusAction}}, "Reboot": {{Type: opv1.RebootStatusAction}}, "RestartCrio": {{Type: opv1.RestartStatusAction, Restart: &opv1.RestartService{ServiceName: "crio.service"}}}, - "ReloadCrio": {{Type: opv1.ReloadStatusAction, Reload: &opv1.ReloadService{ServiceName: "crio.service"}}}} + "ReloadCrio": {{Type: opv1.ReloadStatusAction, Reload: &opv1.ReloadService{ServiceName: "crio.service"}}}, + } tests := []struct { diffPath string @@ -822,7 +830,6 @@ func TestFindClosestFilePolicyPathMatch(t *testing.T) { for idx, test := range tests { t.Run(fmt.Sprintf("case#%d", idx), func(t *testing.T) { - pathFound, actionsFound := ctrlcommon.FindClosestFilePolicyPathMatch(test.diffPath, test.filePolicies) if !reflect.DeepEqual(test.expectedPathFound, pathFound) { @@ -1045,3 +1052,43 @@ func TestLegacyExtensionPackageUpgradeScenario(t *testing.T) { require.True(t, foundNewMatch, "Should find a valid package set matching the new OS image state (current packages)") } + +func TestApplyCryptoPolicy(t *testing.T) { + tests := []struct { + name string + diffFileSet []string + expectError bool + }{ + { + name: "no crypto-policy files in diff is a no-op", + diffFileSet: []string{"/etc/kubernetes/kubelet.conf", "/var/lib/kubelet/config.json"}, + expectError: false, + }, + { + name: "empty diff is a no-op", + diffFileSet: []string{}, + expectError: false, + }, + { + name: "crypto-policy config in diff triggers update", + diffFileSet: []string{"/etc/crypto-policies/config"}, + expectError: true, // update-crypto-policies binary won't exist in test env + }, + { + name: "sub-policy module in diff triggers update", + diffFileSet: []string{"/etc/crypto-policies/policies/modules/TLS13ONLY.pmod"}, + expectError: true, // update-crypto-policies binary won't exist in test env + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := applyCryptoPolicy(tt.diffFileSet) + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/templates/common/_base/files/etc-crypto-policies-config.yaml b/templates/common/_base/files/etc-crypto-policies-config.yaml new file mode 100644 index 0000000000..b034be5320 --- /dev/null +++ b/templates/common/_base/files/etc-crypto-policies-config.yaml @@ -0,0 +1,5 @@ +mode: 0644 +path: "/etc/crypto-policies/config" +contents: + inline: | + {{ .CryptoPolicy }} diff --git a/templates/common/_base/files/etc-crypto-policies-modules-openshift-pmod.yaml b/templates/common/_base/files/etc-crypto-policies-modules-openshift-pmod.yaml new file mode 100644 index 0000000000..59d0d613ee --- /dev/null +++ b/templates/common/_base/files/etc-crypto-policies-modules-openshift-pmod.yaml @@ -0,0 +1,7 @@ +{{- if .CryptoPolicySubMod }} +mode: 0644 +path: "/etc/crypto-policies/policies/modules/OPENSHIFT.pmod" +contents: + inline: | +{{indent 4 .CryptoPolicySubMod}} +{{- end }}