Skip to content

Preparatory patches for bumping fcct -> butane #2980

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-robot merged 5 commits into from
Mar 8, 2022
Merged

Preparatory patches for bumping fcct -> butane #2980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
46a7bce
controller: Add unit test for built-in templating
cgwalters Mar 1, 2022
d44202b
Move helper to read Ignition file contents into controller/common
cgwalters Mar 2, 2022
918f97b
controller: Convert all things reading files from Ignition to use helper
cgwalters Mar 2, 2022
8d66925
helpers: Use a more descriptive variable name
cgwalters Mar 3, 2022
077b97f
controller/tests/kubelet: More consistently check errors
cgwalters Mar 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions pkg/controller/bootstrap/bootstrap_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
ign3types "github.com/coreos/ignition/v2/config/v3_2/types"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/vincent-petithory/dataurl"
"k8s.io/apimachinery/pkg/util/diff"

mcoResourceRead "github.com/openshift/machine-config-operator/lib/resourceread"
Expand Down Expand Up @@ -190,15 +189,15 @@ func TestBootstrapRun(t *testing.T) {
}
}
require.NotNil(t, registriesConfig)
dataURL, err := dataurl.DecodeString(*registriesConfig.Contents.Source)
contents, err := ctrlcommon.DecodeIgnitionFileContents(registriesConfig.Contents.Source, registriesConfig.Contents.Compression)
require.NoError(t, err)
// Only a minimal presence check; more comprehensive tests that the contents correspond to the ICSP semantics are
// maintained in pkg/controller/container-runtime-config.
assert.Contains(t, string(dataURL.Data), "registry.mirror.example.com/ocp")
assert.Contains(t, string(dataURL.Data), "insecure-reg-1.io")
assert.Contains(t, string(dataURL.Data), "insecure-reg-2.io")
assert.Contains(t, string(dataURL.Data), "blocked-reg.io")
assert.NotContains(t, string(dataURL.Data), "release-registry.product.example.org")
assert.Contains(t, string(contents), "registry.mirror.example.com/ocp")
assert.Contains(t, string(contents), "insecure-reg-1.io")
assert.Contains(t, string(contents), "insecure-reg-2.io")
assert.Contains(t, string(contents), "blocked-reg.io")
assert.NotContains(t, string(contents), "release-registry.product.example.org")
})
}
}
49 changes: 44 additions & 5 deletions pkg/controller/common/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package common

import (
"bytes"
"compress/gzip"
"context"
"fmt"
"io"
"io/ioutil"
"net/url"
"os"
Expand Down Expand Up @@ -370,6 +373,42 @@ func validateIgn3FileModes(cfg ign3types.Config) error {
return nil
}

// DecodeIgnitionFileContents returns uncompressed, decoded inline file contents.
// This function does not handle remote resources; it assumes they have already
// been fetched.
func DecodeIgnitionFileContents(source, compression *string) ([]byte, error) {
var contentsBytes []byte

// To allow writing of "empty" files we'll allow source to be nil
if source != nil {
source, err := dataurl.DecodeString(*source)
if err != nil {
return []byte{}, fmt.Errorf("could not decode file content string: %w", err)
}
if compression != nil {
switch *compression {
case "":
contentsBytes = source.Data
case "gzip":
reader, err := gzip.NewReader(bytes.NewReader(source.Data))
if err != nil {
return []byte{}, fmt.Errorf("could not create gzip reader: %w", err)
}
defer reader.Close()
contentsBytes, err = io.ReadAll(reader)
if err != nil {
return []byte{}, fmt.Errorf("failed decompressing: %w", err)
}
default:
return []byte{}, fmt.Errorf("unsupported compression type %q", *compression)
}
} else {
contentsBytes = source.Data
}
}
return contentsBytes, nil
}

// InSlice search for an element in slice and return true if found, otherwise return false
func InSlice(elem string, slice []string) bool {
for _, k := range slice {
Expand Down Expand Up @@ -560,10 +599,10 @@ func TranspileCoreOSConfigToIgn(files, units []string) (*ign3types.Config, error
overwrite := true
outConfig := ign3types.Config{}
// Convert data to Ignition resources
for _, d := range files {
for _, contents := range files {
f := new(fcctbase.File)
if err := yaml.Unmarshal([]byte(d), f); err != nil {
return nil, fmt.Errorf("failed to unmarshal file into struct: %v", err)
if err := yaml.Unmarshal([]byte(contents), f); err != nil {
return nil, fmt.Errorf("failed to unmarshal %q into struct: %v", contents, err)
}
f.Overwrite = &overwrite

Expand All @@ -578,9 +617,9 @@ func TranspileCoreOSConfigToIgn(files, units []string) (*ign3types.Config, error
outConfig = ign3.Merge(outConfig, ign3_2config)
}

for _, d := range units {
for _, contents := range units {
u := new(fcctbase.Unit)
if err := yaml.Unmarshal([]byte(d), u); err != nil {
if err := yaml.Unmarshal([]byte(contents), u); err != nil {
return nil, fmt.Errorf("failed to unmarshal systemd unit into struct: %v", err)
}

Expand Down
54 changes: 53 additions & 1 deletion pkg/controller/common/helpers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ package common

import (
"reflect"
"strings"
"testing"

"github.com/clarketm/json"
ign2types "github.com/coreos/ignition/config/v2_2/types"
ign3types "github.com/coreos/ignition/v2/config/v3_2/types"
validate3 "github.com/coreos/ignition/v2/config/validate"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -15,6 +17,56 @@ import (
"github.com/openshift/machine-config-operator/test/helpers"
)

func TestTranspileCoreOSConfig(t *testing.T) {
kubeletConfig := `
mode: 0644
path: "/etc/kubernetes/kubelet.conf"
contents:
inline: |
kind: KubeletConfiguration
apiVersion: kubelet.config.k8s.io/v1beta1
`
auditConfig := `
mode: 0644
path: "/etc/audit/rules.d/mco-audit.rules"
contents:
inline: |
-a exclude,always -F msgtype=NETFILTER_CFG
`
kubeletService := `name: kubelet.service
enabled: true
contents: |
[Unit]
Description=kubelet
[Service]
ExecStart=/usr/bin/hyperkube
`
crioDropin := `
name: crio.service
dropins:
- name: 10-mco-default-madv.conf
contents: |
[Service]
Environment="GODEBUG=x509ignoreCN=0,madvdontneed=1"
`
dockerDropin := `
name: docker.socket
dropins:
- name: mco-disabled.conf
contents: |
[Unit]
ConditionPathExists=/enoent
`
config, err := TranspileCoreOSConfigToIgn([]string{kubeletConfig, auditConfig}, []string{kubeletService, crioDropin, dockerDropin})
require.Nil(t, err)
if report := validate3.ValidateWithContext(config, nil); report.IsFatal() {
t.Fatalf("invalid ignition V3 config found: %v", report)
}
require.Equal(t, len(config.Storage.Files), 2)
require.True(t, strings.HasPrefix(*config.Storage.Files[0].Contents.Source, "data:,kind%3A%20KubeletConfiguration%0Aapi"))
require.Equal(t, len(config.Systemd.Units), 3)
}

func TestValidateIgnition(t *testing.T) {
// Test that an empty ignition config returns nil
testIgn2Config := ign2types.Config{}
Expand Down Expand Up @@ -248,7 +300,7 @@ func TestMergeMachineConfigs(t *testing.T) {
},
Passwd: ign3types.Passwd{
Users: []ign3types.PasswdUser{
ign3types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ign3types.SSHAuthorizedKey{"1234"}},
{Name: "core", SSHAuthorizedKeys: []ign3types.SSHAuthorizedKey{"1234"}},
},
},
}
Expand Down
13 changes: 6 additions & 7 deletions pkg/controller/container-runtime-config/container_runtime_config_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
cligolistersv1 "github.com/openshift/client-go/config/listers/config/v1"
operatorinformersv1alpha1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1alpha1"
operatorlistersv1alpha1 "github.com/openshift/client-go/operator/listers/operator/v1alpha1"
"github.com/vincent-petithory/dataurl"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -659,11 +658,11 @@ func mergeConfigChanges(origFile *ign3types.File, cfg *mcfgv1.ContainerRuntimeCo
if origFile.Contents.Source == nil {
return nil, fmt.Errorf("original Container Runtime config is empty")
}
dataURL, err := dataurl.DecodeString(*origFile.Contents.Source)
contents, err := ctrlcommon.DecodeIgnitionFileContents(origFile.Contents.Source, origFile.Contents.Compression)
if err != nil {
return nil, fmt.Errorf("could not decode original Container Runtime config: %v", err)
}
cfgTOML, err := update(dataURL.Data, cfg.Spec.ContainerRuntimeConfig)
cfgTOML, err := update(contents, cfg.Spec.ContainerRuntimeConfig)
if err != nil {
return nil, fmt.Errorf("could not update container runtime config with new changes: %v", err)
}
Expand Down Expand Up @@ -824,11 +823,11 @@ func registriesConfigIgnition(templateDir string, controllerConfig *mcfgv1.Contr
if originalRegistriesIgn.Contents.Source == nil {
return nil, fmt.Errorf("original registries config is empty")
}
dataURL, err := dataurl.DecodeString(*originalRegistriesIgn.Contents.Source)
contents, err := ctrlcommon.DecodeIgnitionFileContents(originalRegistriesIgn.Contents.Source, originalRegistriesIgn.Contents.Compression)
if err != nil {
return nil, fmt.Errorf("could not decode original registries config: %v", err)
}
registriesTOML, err = updateRegistriesConfig(dataURL.Data, insecureRegs, blockedRegs, icspRules)
registriesTOML, err = updateRegistriesConfig(contents, insecureRegs, blockedRegs, icspRules)
if err != nil {
return nil, fmt.Errorf("could not update registries config with new changes: %v", err)
}
Expand All @@ -837,11 +836,11 @@ func registriesConfigIgnition(templateDir string, controllerConfig *mcfgv1.Contr
if originalPolicyIgn.Contents.Source == nil {
return nil, fmt.Errorf("original policy json is empty")
}
dataURL, err := dataurl.DecodeString(*originalPolicyIgn.Contents.Source)
contents, err := ctrlcommon.DecodeIgnitionFileContents(originalPolicyIgn.Contents.Source, originalPolicyIgn.Contents.Compression)
if err != nil {
return nil, fmt.Errorf("could not decode original policy json: %v", err)
}
policyJSON, err = updatePolicyJSON(dataURL.Data, blockedRegs, allowedRegs)
policyJSON, err = updatePolicyJSON(contents, blockedRegs, allowedRegs)
if err != nil {
return nil, fmt.Errorf("could not update policy json with new changes: %v", err)
}
Expand Down
13 changes: 6 additions & 7 deletions pkg/controller/container-runtime-config/container_runtime_config_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/golang/glog"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/vincent-petithory/dataurl"

"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -391,9 +390,9 @@ func verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mc *mcfgv1.Machin
regfile = ignCfg.Storage.Files[1]
}
assert.Equal(t, registriesConfigPath, regfile.Node.Path)
registriesConf, err := dataurl.DecodeString(*regfile.Contents.Source)
registriesConf, err := ctrlcommon.DecodeIgnitionFileContents(regfile.Contents.Source, regfile.Contents.Compression)
require.NoError(t, err)
assert.Equal(t, string(expectedRegistriesConf), string(registriesConf.Data))
assert.Equal(t, string(expectedRegistriesConf), string(registriesConf))

// Validate the policy.json contents if a change is expected from the tests
if verifyPolicyJSON {
Expand All @@ -406,9 +405,9 @@ func verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mc *mcfgv1.Machin
policyfile = ignCfg.Storage.Files[0]
}
assert.Equal(t, policyConfigPath, policyfile.Node.Path)
policyJSON, err := dataurl.DecodeString(*policyfile.Contents.Source)
policyJSON, err := ctrlcommon.DecodeIgnitionFileContents(policyfile.Contents.Source, policyfile.Contents.Compression)
require.NoError(t, err)
assert.Equal(t, string(expectedPolicyJSON), string(policyJSON.Data))
assert.Equal(t, string(expectedPolicyJSON), string(policyJSON))
}

// Validate the drop-in search registries file contents if a change is expected from the tests
Expand All @@ -419,9 +418,9 @@ func verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mc *mcfgv1.Machin
dropinfile = ignCfg.Storage.Files[1]
}
assert.Equal(t, searchRegDropInFilePath, dropinfile.Node.Path)
searchRegsConf, err := dataurl.DecodeString(*dropinfile.Contents.Source)
searchRegsConf, err := ctrlcommon.DecodeIgnitionFileContents(dropinfile.Contents.Source, dropinfile.Contents.Compression)
require.NoError(t, err)
assert.Equal(t, string(expectedSearchRegsConf[0].data), string(searchRegsConf.Data))
assert.Equal(t, string(expectedSearchRegsConf[0].data), string(searchRegsConf))
}
}

Expand Down
5 changes: 2 additions & 3 deletions pkg/controller/kubelet-config/kubelet_config_bootstrap_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
"github.com/openshift/machine-config-operator/test/helpers"
"github.com/stretchr/testify/require"
"github.com/vincent-petithory/dataurl"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1"
Expand Down Expand Up @@ -91,9 +90,9 @@ func verifyKubeletConfigJSONContents(t *testing.T, mc *mcfgv1.MachineConfig, mcN
ignCfg, err := ctrlcommon.ParseAndConvertConfig(mc.Spec.Config.Raw)
require.NoError(t, err)
regfile := ignCfg.Storage.Files[0]
conf, err := dataurl.DecodeString(*regfile.Contents.Source)
conf, err := ctrlcommon.DecodeIgnitionFileContents(regfile.Contents.Source, regfile.Contents.Compression)
require.NoError(t, err)
require.Contains(t, string(conf.Data), `"maxPods": 100`)
require.Contains(t, string(conf), `"maxPods": 100`)
}

func TestGenerateDefaultManagedKeyKubelet(t *testing.T) {
Expand Down
5 changes: 2 additions & 3 deletions pkg/controller/kubelet-config/kubelet_config_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
ign3types "github.com/coreos/ignition/v2/config/v3_2/types"
"github.com/golang/glog"
"github.com/imdario/mergo"
"github.com/vincent-petithory/dataurl"
corev1 "k8s.io/api/core/v1"
macherrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -340,11 +339,11 @@ func generateOriginalKubeletConfigWithFeatureGates(cc *mcfgv1.ControllerConfig,
if originalKubeletIgn.Contents.Source == nil {
return nil, fmt.Errorf("the original Kubelet source string is empty: %v", err)
}
dataURL, err := dataurl.DecodeString(*originalKubeletIgn.Contents.Source)
contents, err := ctrlcommon.DecodeIgnitionFileContents(originalKubeletIgn.Contents.Source, originalKubeletIgn.Contents.Compression)
if err != nil {
return nil, fmt.Errorf("could not decode the original Kubelet source string: %v", err)
}
originalKubeConfig, err := decodeKubeletConfig(dataURL.Data)
originalKubeConfig, err := decodeKubeletConfig(contents)
if err != nil {
return nil, fmt.Errorf("could not deserialize the Kubelet source: %v", err)
}
Expand Down
24 changes: 15 additions & 9 deletions pkg/controller/kubelet-config/kubelet_config_features_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
ign3types "github.com/coreos/ignition/v2/config/v3_2/types"
configv1 "github.com/openshift/api/config/v1"
osev1 "github.com/openshift/api/config/v1"
"github.com/vincent-petithory/dataurl"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1"

Expand All @@ -29,8 +28,10 @@ func TestFeatureGateDrift(t *testing.T) {
if err != nil {
t.Errorf("could not generate kubelet config from templates %v", err)
}
dataURL, _ := dataurl.DecodeString(*kubeletConfig.Contents.Source)
originalKubeConfig, _ := decodeKubeletConfig(dataURL.Data)
contents, err := ctrlcommon.DecodeIgnitionFileContents(kubeletConfig.Contents.Source, kubeletConfig.Contents.Compression)
require.NoError(t, err)
originalKubeConfig, err := decodeKubeletConfig(contents)
require.NoError(t, err)
defaultFeatureGates, err := generateFeatureMap(createNewDefaultFeatureGate())
if err != nil {
t.Errorf("could not generate defaultFeatureGates: %v", err)
Expand All @@ -53,8 +54,10 @@ func TestFeaturesDefault(t *testing.T) {
mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0")
kc1 := newKubeletConfig("smaller-max-pods", &kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 100}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "pools.operator.machineconfiguration.openshift.io/master", ""))
kc2 := newKubeletConfig("bigger-max-pods", &kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 250}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "pools.operator.machineconfiguration.openshift.io/master", ""))
kubeletConfigKey1, _ := getManagedKubeletConfigKey(mcp, f.client, kc1)
kubeletConfigKey2, _ := getManagedKubeletConfigKey(mcp2, f.client, kc2)
kubeletConfigKey1, err := getManagedKubeletConfigKey(mcp, f.client, kc1)
require.NoError(t, err)
kubeletConfigKey2, err := getManagedKubeletConfigKey(mcp2, f.client, kc2)
require.NoError(t, err)
mcs := helpers.NewMachineConfig(kubeletConfigKey1, map[string]string{"node-role/master": ""}, "dummy://", []ign3types.File{{}})
mcs2 := helpers.NewMachineConfig(kubeletConfigKey2, map[string]string{"node-role/worker": ""}, "dummy://", []ign3types.File{{}})
mcsDeprecated := mcs.DeepCopy()
Expand Down Expand Up @@ -92,8 +95,10 @@ func TestFeaturesCustomNoUpgrade(t *testing.T) {
mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0")
kc1 := newKubeletConfig("smaller-max-pods", &kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 100}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "pools.operator.machineconfiguration.openshift.io/master", ""))
kc2 := newKubeletConfig("bigger-max-pods", &kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 250}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "pools.operator.machineconfiguration.openshift.io/master", ""))
kubeletConfigKey1, _ := getManagedKubeletConfigKey(mcp, f.client, kc1)
kubeletConfigKey2, _ := getManagedKubeletConfigKey(mcp2, f.client, kc2)
kubeletConfigKey1, err := getManagedKubeletConfigKey(mcp, f.client, kc1)
require.NoError(t, err)
kubeletConfigKey2, err := getManagedKubeletConfigKey(mcp2, f.client, kc2)
require.NoError(t, err)
mcs := helpers.NewMachineConfig(kubeletConfigKey1, map[string]string{"node-role/master": ""}, "dummy://", []ign3types.File{{}})
mcs2 := helpers.NewMachineConfig(kubeletConfigKey2, map[string]string{"node-role/worker": ""}, "dummy://", []ign3types.File{{}})
mcsDeprecated := mcs.DeepCopy()
Expand Down Expand Up @@ -190,10 +195,11 @@ func TestBootstrapFeaturesCustomNoUpgrade(t *testing.T) {
for _, mc := range mcs {
ignCfg, err := ctrlcommon.ParseAndConvertConfig(mc.Spec.Config.Raw)
regfile := ignCfg.Storage.Files[0]
conf, err := dataurl.DecodeString(*regfile.Contents.Source)
conf, err := ctrlcommon.DecodeIgnitionFileContents(regfile.Contents.Source, regfile.Contents.Compression)
require.NoError(t, err)

originalKubeConfig, _ := decodeKubeletConfig(conf.Data)
originalKubeConfig, err := decodeKubeletConfig(conf)
require.NoError(t, err)
defaultFeatureGates, err := generateFeatureMap(createNewDefaultFeatureGate())
if err != nil {
t.Errorf("could not generate defaultFeatureGates: %v", err)
Expand Down
Loading