diff --git a/go.mod b/go.mod index 9e98a51337..0f0c1cca3a 100644 --- a/go.mod +++ b/go.mod @@ -80,6 +80,7 @@ require ( replace ( github.com/InVisionApp/go-health => github.com/InVisionApp/go-health v1.1.7-0.20190926150048-b5cab38233bb + github.com/coreos/ign-converter => github.com/LorbusChris/ign-converter v0.0.0-20200701232648-56880ed0a25d github.com/go-log/log => github.com/go-log/log v0.1.1-0.20181211034820-a514cf01a3eb github.com/godbus/dbus => github.com/godbus/dbus v0.0.0-20190623212516-8a1682060722 github.com/opencontainers/runtime-spec => github.com/opencontainers/runtime-spec v0.1.2-0.20190408193819-a1b50f621a48 diff --git a/go.sum b/go.sum index 9beb020040..ac9a7e5f95 100644 --- a/go.sum +++ b/go.sum @@ -20,6 +20,8 @@ github.com/InVisionApp/go-health v1.1.7-0.20190926150048-b5cab38233bb h1:hWMXKLe github.com/InVisionApp/go-health v1.1.7-0.20190926150048-b5cab38233bb/go.mod h1:l1F5lzgPxAQwAPIrj5HJT+pWj9gfX1uMFWM/Y2gCHcU= github.com/InVisionApp/go-logger v1.0.1 h1:WFL19PViM1mHUmUWfsv5zMo379KSWj2MRmBlzMFDRiE= github.com/InVisionApp/go-logger v1.0.1/go.mod h1:+cGTDSn+P8105aZkeOfIhdd7vFO5X1afUHcjvanY0L8= +github.com/LorbusChris/ign-converter v0.0.0-20200701232648-56880ed0a25d h1:mexPxjDu43XhrPAxXjvRm0Oe3WLkswINjXS7JPi5JMU= +github.com/LorbusChris/ign-converter v0.0.0-20200701232648-56880ed0a25d/go.mod h1:LNu0WTt8iVH/WJH15R/SjZw7AdyY2qAyf9ILZTCBvho= github.com/MakeNowJust/heredoc v0.0.0-20170808103936-bb23615498cd h1:sjQovDkwrZp8u+gxLtPgKGjk5hCxuy2hrRejBTA9xFU= github.com/MakeNowJust/heredoc v0.0.0-20170808103936-bb23615498cd/go.mod h1:64YHyfSL2R96J44Nlwm39UHepQbyR5q10x7iYa1ks2E= github.com/Masterminds/goutils v1.1.0 h1:zukEsf/1JZwCMgHiK3GZftabmxiCw4apj3a28RPBiVg= @@ -114,8 +116,6 @@ github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f h1:JOrtw2xFKzlg+ github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= github.com/coreos/go-systemd/v22 v22.0.0 h1:XJIw/+VlJ+87J+doOxznsAWIdmWuViOVhkQamW5YV28= github.com/coreos/go-systemd/v22 v22.0.0/go.mod h1:xO0FLkIi5MaZafQlIrOotqXZ90ih+1atmu1JpKERPPk= -github.com/coreos/ign-converter v0.0.0-20200629171308-e40a44f244c5 h1:rBga8xIJ7MzGbTfruI8Kfxu1X3FlIquMRuW9yLsW5Mc= -github.com/coreos/ign-converter v0.0.0-20200629171308-e40a44f244c5/go.mod h1:LNu0WTt8iVH/WJH15R/SjZw7AdyY2qAyf9ILZTCBvho= github.com/coreos/ignition v0.33.0 h1:rYJoGv5v/5rCJAzyMaE9gU8pn7w7pv0M4rDzHvDK6T4= github.com/coreos/ignition v0.33.0/go.mod h1:WJQapxzEn9DE0ryxsGvm8QnBajm/XsS/PkrDqSpz+bA= github.com/coreos/ignition v0.35.0 h1:UFodoYq1mOPrbEjtxIsZbThcDyQwAI1owczRDqWmKkQ= diff --git a/lib/resourceapply/machineconfig_test.go b/lib/resourceapply/machineconfig_test.go index 9feb0b59f8..27468987d1 100644 --- a/lib/resourceapply/machineconfig_test.go +++ b/lib/resourceapply/machineconfig_test.go @@ -4,18 +4,24 @@ import ( "fmt" "testing" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" "github.com/davecgh/go-spew/spew" - mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" - "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/fake" - "github.com/openshift/machine-config-operator/test/helpers" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/diff" clienttesting "k8s.io/client-go/testing" + + mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" + "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/fake" + "github.com/openshift/machine-config-operator/test/helpers" ) +// Golang :cry: +func strToPtr(s string) *string { + return &s +} + func TestApplyMachineConfig(t *testing.T) { tests := []struct { existing []runtime.Object @@ -180,7 +186,7 @@ func TestApplyMachineConfig(t *testing.T) { Raw: helpers.MarshalOrDie(&igntypes.Config{ Passwd: igntypes.Passwd{ Users: []igntypes.PasswdUser{{ - HomeDir: "/home/dummy", + HomeDir: strToPtr("/home/dummy"), }}, }, }), @@ -206,7 +212,7 @@ func TestApplyMachineConfig(t *testing.T) { Raw: helpers.MarshalOrDie(&igntypes.Config{ Passwd: igntypes.Passwd{ Users: []igntypes.PasswdUser{{ - HomeDir: "/home/dummy", + HomeDir: strToPtr("/home/dummy"), }}, }, }), @@ -227,7 +233,7 @@ func TestApplyMachineConfig(t *testing.T) { Raw: helpers.MarshalOrDie(&igntypes.Config{ Passwd: igntypes.Passwd{ Users: []igntypes.PasswdUser{{ - HomeDir: "/home/dummy-prev", + HomeDir: strToPtr("/home/dummy-prev"), }}, }, }), @@ -242,7 +248,7 @@ func TestApplyMachineConfig(t *testing.T) { Raw: helpers.MarshalOrDie(&igntypes.Config{ Passwd: igntypes.Passwd{ Users: []igntypes.PasswdUser{{ - HomeDir: "/home/dummy", + HomeDir: strToPtr("/home/dummy"), }}, }, }), @@ -268,7 +274,7 @@ func TestApplyMachineConfig(t *testing.T) { Raw: helpers.MarshalOrDie(&igntypes.Config{ Passwd: igntypes.Passwd{ Users: []igntypes.PasswdUser{{ - HomeDir: "/home/dummy", + HomeDir: strToPtr("/home/dummy"), }}, }, }), diff --git a/pkg/controller/bootstrap/bootstrap_test.go b/pkg/controller/bootstrap/bootstrap_test.go index 8102a104a3..d4c6be6753 100644 --- a/pkg/controller/bootstrap/bootstrap_test.go +++ b/pkg/controller/bootstrap/bootstrap_test.go @@ -9,7 +9,7 @@ import ( "strings" "testing" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/vincent-petithory/dataurl" @@ -159,7 +159,7 @@ func TestBootstrapRun(t *testing.T) { } } require.NotNil(t, registriesConfig) - dataURL, err := dataurl.DecodeString(registriesConfig.Contents.Source) + dataURL, err := dataurl.DecodeString(*registriesConfig.Contents.Source) 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/continer-runtime-config. diff --git a/pkg/controller/common/helpers.go b/pkg/controller/common/helpers.go index 4ab8dd3bc6..dd7b6c46ff 100644 --- a/pkg/controller/common/helpers.go +++ b/pkg/controller/common/helpers.go @@ -11,7 +11,6 @@ import ( fcctbase "github.com/coreos/fcct/base/v0_1" "github.com/coreos/ign-converter/translate/v23tov30" "github.com/coreos/ign-converter/translate/v31tov22" - ign2error "github.com/coreos/ignition/config/shared/errors" ign2 "github.com/coreos/ignition/config/v2_2" ign2types "github.com/coreos/ignition/config/v2_2/types" ign2_3 "github.com/coreos/ignition/config/v2_3" @@ -33,6 +32,11 @@ import ( mcfgclientset "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned" ) +// StrToPtr is a helper function to return a pointer to a string +func StrToPtr(s string) *string { + return &s +} + // MergeMachineConfigs combines multiple machineconfig objects into one object. // It sorts all the configs in increasing order of their name. // It uses the Ignition config from first object as base and appends all the rest. @@ -46,11 +50,11 @@ func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, osImageURL string) (*m var fips bool var kernelType string - var outIgn ign2types.Config + var outIgn ign3types.Config var err error if configs[0].Spec.Config.Raw == nil { - outIgn = ign2types.Config{} + outIgn = ign3types.Config{} } else { outIgn, err = IgnParseWrapper(configs[0].Spec.Config.Raw) if err != nil { @@ -64,16 +68,16 @@ func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, osImageURL string) (*m fips = true } - var appendIgn ign2types.Config + var appendIgn ign3types.Config if configs[idx].Spec.Config.Raw == nil { - appendIgn = ign2types.Config{} + appendIgn = ign3types.Config{} } else { appendIgn, err = IgnParseWrapper(configs[idx].Spec.Config.Raw) if err != nil { return nil, err } } - outIgn = ign2.Append(outIgn, appendIgn) + outIgn = ign3.Merge(outIgn, appendIgn) } rawOutIgn, err := json.Marshal(outIgn) if err != nil { @@ -115,10 +119,10 @@ func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, osImageURL string) (*m } // NewIgnConfig returns an empty ignition config with version set as latest version -func NewIgnConfig() ign2types.Config { - return ign2types.Config{ - Ignition: ign2types.Ignition{ - Version: ign2types.MaxVersion.String(), +func NewIgnConfig() ign3types.Config { + return ign3types.Config{ + Ignition: ign3types.Ignition{ + Version: ign3types.MaxVersion.String(), }, } } @@ -130,16 +134,16 @@ func WriteTerminationError(err error) { glog.Fatal(msg) } -// ConvertRawExtIgnition2to3 converts a RawExtension containing Ignition spec v2.2 config -// into a RawExtension containing Ignition spec v3.1 config -func ConvertRawExtIgnition2to3(inRawExtIgnV2 *runtime.RawExtension) (runtime.RawExtension, error) { - ignCfg, rpt, err := ign2.Parse(inRawExtIgnV2.Raw) +// ConvertRawExtIgnition3to2 converts a RawExtension containing Ignition spec v3.1 config +// into a RawExtension containing Ignition spec v2.2 config +func ConvertRawExtIgnition3to2(inRawExtIgnV3 *runtime.RawExtension) (runtime.RawExtension, error) { + ignCfg, rpt, err := ign3.Parse(inRawExtIgnV3.Raw) if err != nil || rpt.IsFatal() { - return runtime.RawExtension{}, errors.Errorf("parsing Ignition config spec v2.2 failed with error: %v\nReport: %v", err, rpt) + return runtime.RawExtension{}, errors.Errorf("parsing Ignition config spec v3.1 failed with error: %v\nReport: %v", err, rpt) } - converted3, err := convertIgnition2to3(ignCfg) + converted3, err := convertIgnition3to2(ignCfg) if err != nil { - return runtime.RawExtension{}, errors.Errorf("failed to convert config from spec v2.2 to v3.1: %v", err) + return runtime.RawExtension{}, errors.Errorf("failed to convert config from spec v3.1 to v2.2: %v", err) } outIgnV3, err := json.Marshal(converted3) @@ -233,42 +237,39 @@ func ValidateMachineConfig(cfg mcfgv1.MachineConfigSpec) error { // IgnParseWrapper parses rawIgn for v2.2, v3.1 and v3.0 Ignition configs and returns // a spec v2.2 or an error // This wrapper is necessary since each version uses a different parser. -func IgnParseWrapper(rawIgn []byte) (ign2types.Config, error) { - ignCfg, rpt, err := ign2.Parse(rawIgn) +func IgnParseWrapper(rawIgn []byte) (ign3types.Config, error) { + ignCfg, rpt, err := ign3.Parse(rawIgn) if err == nil && !rpt.IsFatal() { - // this is an ign spec v2.2 config that was successfully parsed + // this is an ign spec v3.1 config that was successfully parsed return ignCfg, nil } - if err.Error() == ign2error.ErrUnknownVersion.Error() { - // check to see if this is ign config spec v3.1 - ignCfgV3, rptV3, errV3 := ign3.Parse(rawIgn) - if errV3 == nil && !rptV3.IsFatal() { - convertedIgnV2, err := convertIgnition3to2(ignCfgV3) - if err != nil { - return ign2types.Config{}, errors.Errorf("failed to convert Ignition config spec v3 to v2: %v", err) - } - return convertedIgnV2, nil - - } else if errV3.Error() == ign3error.ErrUnknownVersion.Error() { - // unlike spec v2.x parsers, v3.x parsers aren't chained by default, - // so try with spec v3.0 parser as well - ignCfgV3_0, rptV3_0, errV3_0 := ign3_0.Parse(rawIgn) - if errV3_0 == nil && !rptV3_0.IsFatal() { - convertedIgnV2, err := convertIgnition3to2(translate3.Translate(ignCfgV3_0)) + // unlike spec v2.x parsers, v3.x parsers aren't chained by default, + if err.Error() == ign3error.ErrUnknownVersion.Error() { + // check if this is ign config spec v3.0 + ignCfgV3_0, rptV3_0, errV3_0 := ign3_0.Parse(rawIgn) + if errV3_0 == nil && !rptV3_0.IsFatal() { + return translate3.Translate(ignCfgV3_0), nil + } + if errV3_0.Error() == ign3error.ErrUnknownVersion.Error() { + // check if this is ign config spec v2.2 + ignCfgV2_2, rptV2_2, errV2_2 := ign2.Parse(rawIgn) + if errV2_2 == nil && !rptV2_2.IsFatal() { + // now convert to spec v3.1 + convertedIgnV3, err := convertIgnition2to3(ignCfgV2_2) if err != nil { - return ign2types.Config{}, errors.Errorf("failed to convert Ignition config spec v3 to v2: %v", err) + return ign3types.Config{}, errors.Errorf("failed to convert Ignition config spec v2.2 to v3.1: %v", err) } - return convertedIgnV2, nil + return convertedIgnV3, nil } - return ign2types.Config{}, errors.Errorf("parsing Ignition config spec v3.0 failed with error: %v\nReport: %v", errV3_0, rptV3_0) + return ign3types.Config{}, errors.Errorf("parsing Ignition config spec v2.2 failed with error: %v\nReport: %v", errV2_2, rptV2_2) } - return ign2types.Config{}, errors.Errorf("parsing Ignition config spec v3.1 failed with error: %v\nReport: %v", errV3, rptV3) + return ign3types.Config{}, errors.Errorf("parsing Ignition config spec v3.0 failed with error: %v\nReport: %v", errV3_0, rptV3_0) } - return ign2types.Config{}, errors.Errorf("parsing Ignition config spec v2 failed with error: %v\nReport: %v", err, rpt) + return ign3types.Config{}, errors.Errorf("parsing Ignition config spec v3.1 failed with error: %v\nReport: %v", err, rpt) } // TranspileCoreOSConfigToIgn transpiles Fedora CoreOS config to ignition diff --git a/pkg/controller/common/helpers_test.go b/pkg/controller/common/helpers_test.go index 5d657a38e2..c0f9c9dce3 100644 --- a/pkg/controller/common/helpers_test.go +++ b/pkg/controller/common/helpers_test.go @@ -90,38 +90,40 @@ func TestConvertIgnition3to2(t *testing.T) { func TestIgnParseWrapper(t *testing.T) { // Make a new Ign3.1 config - testIgn3Config := ign3types.Config{} + testIgn3Config := NewIgnConfig() tempUser := ign3types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ign3types.SSHAuthorizedKey{"5678", "abc"}} testIgn3Config.Passwd.Users = []ign3types.PasswdUser{tempUser} testIgn3Config.Ignition.Version = "3.1.0" // Make a Ign2 comp config - testIgn2Config := NewIgnConfig() + testIgn2Config := ign2types.Config{} tempUser2 := ign2types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ign2types.SSHAuthorizedKey{"5678", "abc"}} testIgn2Config.Passwd.Users = []ign2types.PasswdUser{tempUser2} + testIgn2Config.Ignition.Version = "2.2.0" // turn v2.2 config into a raw []byte rawIgn := helpers.MarshalOrDie(testIgn2Config) // check that it was parsed successfully convertedIgn, err := IgnParseWrapper(rawIgn) require.Nil(t, err) - assert.Equal(t, testIgn2Config, convertedIgn) + assert.Equal(t, testIgn3Config, convertedIgn) // turn v3.1 config into a raw []byte rawIgn = helpers.MarshalOrDie(testIgn3Config) // check that it was parsed successfully convertedIgn, err = IgnParseWrapper(rawIgn) require.Nil(t, err) - assert.Equal(t, testIgn2Config, convertedIgn) + assert.Equal(t, testIgn3Config, convertedIgn) // Make a valid Ign 3.0 cfg - testIgn3Config.Ignition.Version = "3.0.0" + testIgn3_0Config := testIgn3Config + testIgn3_0Config.Ignition.Version = "3.0.0" // turn it into a raw []byte - rawIgn = helpers.MarshalOrDie(testIgn3Config) + rawIgn = helpers.MarshalOrDie(testIgn3_0Config) // check that it was parsed successfully convertedIgn, err = IgnParseWrapper(rawIgn) require.Nil(t, err) - assert.Equal(t, testIgn2Config, convertedIgn) + assert.Equal(t, testIgn3Config, convertedIgn) // Make a bad Ign3 cfg testIgn3Config.Ignition.Version = "21.0.0" @@ -129,5 +131,5 @@ func TestIgnParseWrapper(t *testing.T) { // check that it failed since this is an invalid cfg convertedIgn, err = IgnParseWrapper(rawIgn) require.NotNil(t, err) - assert.Equal(t, ign2types.Config{}, convertedIgn) + assert.Equal(t, ign3types.Config{}, convertedIgn) } diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller.go b/pkg/controller/container-runtime-config/container_runtime_config_controller.go index c0e51f6a70..59d2beda9a 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller.go @@ -7,7 +7,7 @@ import ( "time" "github.com/clarketm/json" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" "github.com/golang/glog" apicfgv1 "github.com/openshift/api/config/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" @@ -597,7 +597,7 @@ func (ctrl *Controller) syncContainerRuntimeConfig(key string) error { // mergeConfigChanges retrieves the original/default config data from the templates, decodes it and merges in the changes given by the Custom Resource. // It then encodes the new data and returns it. func (ctrl *Controller) mergeConfigChanges(origFile *igntypes.File, cfg *mcfgv1.ContainerRuntimeConfig, update updateConfigFunc) ([]byte, error) { - dataURL, err := dataurl.DecodeString(origFile.Contents.Source) + dataURL, err := dataurl.DecodeString(*origFile.Contents.Source) if err != nil { return nil, ctrl.syncStatusOnly(cfg, err, "could not decode original Container Runtime config: %v", err) } @@ -755,7 +755,7 @@ func registriesConfigIgnition(templateDir string, controllerConfig *mcfgv1.Contr } if insecureRegs != nil || blockedRegs != nil || len(icspRules) != 0 { - dataURL, err := dataurl.DecodeString(originalRegistriesIgn.Contents.Source) + dataURL, err := dataurl.DecodeString(*originalRegistriesIgn.Contents.Source) if err != nil { return nil, fmt.Errorf("could not decode original registries config: %v", err) } @@ -765,7 +765,7 @@ func registriesConfigIgnition(templateDir string, controllerConfig *mcfgv1.Contr } } if blockedRegs != nil || allowedRegs != nil { - dataURL, err := dataurl.DecodeString(originalPolicyIgn.Contents.Source) + dataURL, err := dataurl.DecodeString(*originalPolicyIgn.Contents.Source) if err != nil { return nil, fmt.Errorf("could not decode original policy json: %v", err) } diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go index d763747c35..164ef636bf 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go @@ -26,7 +26,7 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" apicfgv1 "github.com/openshift/api/config/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" fakeconfigv1client "github.com/openshift/client-go/config/clientset/versioned/fake" @@ -371,7 +371,7 @@ 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 := dataurl.DecodeString(*regfile.Contents.Source) require.NoError(t, err) assert.Equal(t, string(expectedRegistriesConf), string(registriesConf.Data)) @@ -386,7 +386,7 @@ 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 := dataurl.DecodeString(*policyfile.Contents.Source) require.NoError(t, err) assert.Equal(t, string(expectedPolicyJSON), string(policyJSON.Data)) } diff --git a/pkg/controller/container-runtime-config/helpers.go b/pkg/controller/container-runtime-config/helpers.go index 8625e02573..5d2657ec08 100644 --- a/pkg/controller/container-runtime-config/helpers.go +++ b/pkg/controller/container-runtime-config/helpers.go @@ -12,7 +12,7 @@ import ( "github.com/containers/image/pkg/sysregistriesv2" signature "github.com/containers/image/signature" storageconfig "github.com/containers/storage/pkg/config" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" "github.com/golang/glog" apicfgv1 "github.com/openshift/api/config/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" @@ -113,13 +113,12 @@ func createNewIgnition(configs []generatedConfigFile) igntypes.Config { configdu.Encoding = dataurl.EncodingASCII configTempFile := igntypes.File{ Node: igntypes.Node{ - Filesystem: "root", - Path: ignConf.filePath, + Path: ignConf.filePath, }, FileEmbedded1: igntypes.FileEmbedded1{ Mode: &mode, - Contents: igntypes.FileContents{ - Source: configdu.String(), + Contents: igntypes.Resource{ + Source: ctrlcommon.StrToPtr(configdu.String()), }, }, } diff --git a/pkg/controller/kubelet-config/helpers.go b/pkg/controller/kubelet-config/helpers.go index cfa4a8b28f..af297bd181 100644 --- a/pkg/controller/kubelet-config/helpers.go +++ b/pkg/controller/kubelet-config/helpers.go @@ -5,7 +5,7 @@ import ( "encoding/json" "fmt" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" osev1 "github.com/openshift/api/config/v1" "github.com/vincent-petithory/dataurl" corev1 "k8s.io/api/core/v1" @@ -32,13 +32,12 @@ func createNewKubeletIgnition(jsonConfig []byte) igntypes.Config { du.Encoding = dataurl.EncodingASCII tempFile := igntypes.File{ Node: igntypes.Node{ - Filesystem: "root", - Path: "/etc/kubernetes/kubelet.conf", + Path: "/etc/kubernetes/kubelet.conf", }, FileEmbedded1: igntypes.FileEmbedded1{ Mode: &mode, - Contents: igntypes.FileContents{ - Source: du.String(), + Contents: igntypes.Resource{ + Source: ctrlcommon.StrToPtr(du.String()), }, }, } diff --git a/pkg/controller/kubelet-config/kubelet_config_controller.go b/pkg/controller/kubelet-config/kubelet_config_controller.go index e495d0b7f4..1c88b597e2 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller.go @@ -8,7 +8,7 @@ import ( "time" "github.com/clarketm/json" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" "github.com/golang/glog" "github.com/imdario/mergo" "github.com/vincent-petithory/dataurl" @@ -453,7 +453,7 @@ func (ctrl *Controller) syncKubeletConfig(key string) error { if err != nil { return ctrl.syncStatusOnly(cfg, err, "could not generate the original Kubelet config: %v", err) } - dataURL, err := dataurl.DecodeString(originalKubeletIgn.Contents.Source) + dataURL, err := dataurl.DecodeString(*originalKubeletIgn.Contents.Source) if err != nil { return ctrl.syncStatusOnly(cfg, err, "could not decode the original Kubelet source string: %v", err) } diff --git a/pkg/controller/kubelet-config/kubelet_config_controller_test.go b/pkg/controller/kubelet-config/kubelet_config_controller_test.go index f234ba3e43..adf88ac302 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller_test.go @@ -25,7 +25,7 @@ import ( osev1 "github.com/openshift/api/config/v1" oseinformersv1 "github.com/openshift/client-go/config/informers/externalversions" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" oseconfigfake "github.com/openshift/client-go/config/clientset/versioned/fake" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" diff --git a/pkg/controller/kubelet-config/kubelet_config_features.go b/pkg/controller/kubelet-config/kubelet_config_features.go index 7488e1146c..ecccc428c2 100644 --- a/pkg/controller/kubelet-config/kubelet_config_features.go +++ b/pkg/controller/kubelet-config/kubelet_config_features.go @@ -106,7 +106,7 @@ func (ctrl *Controller) syncFeatureHandler(key string) error { if err != nil { return err } - dataURL, err := dataurl.DecodeString(originalKubeletIgn.Contents.Source) + dataURL, err := dataurl.DecodeString(*originalKubeletIgn.Contents.Source) if err != nil { return err } diff --git a/pkg/controller/kubelet-config/kubelet_config_features_test.go b/pkg/controller/kubelet-config/kubelet_config_features_test.go index a943f7970e..266711b507 100644 --- a/pkg/controller/kubelet-config/kubelet_config_features_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_features_test.go @@ -4,7 +4,7 @@ import ( "reflect" "testing" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" configv1 "github.com/openshift/api/config/v1" "github.com/vincent-petithory/dataurl" @@ -24,7 +24,7 @@ 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) + dataURL, _ := dataurl.DecodeString(*kubeletConfig.Contents.Source) originalKubeConfig, _ := decodeKubeletConfig(dataURL.Data) defaultFeatureGates, err := ctrl.generateFeatureMap(createNewDefaultFeatureGate()) if err != nil { diff --git a/pkg/controller/render/render_controller_test.go b/pkg/controller/render/render_controller_test.go index dab89d6a54..609e4a0ab3 100644 --- a/pkg/controller/render/render_controller_test.go +++ b/pkg/controller/render/render_controller_test.go @@ -7,7 +7,7 @@ import ( "time" "github.com/clarketm/json" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" configv1 "github.com/openshift/api/config/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -275,13 +275,11 @@ func TestIgnValidationGenerateRenderedMachineConfig(t *testing.T) { mcp := helpers.NewMachineConfigPool("test-cluster-master", helpers.MasterSelector, nil, "") files := []igntypes.File{{ Node: igntypes.Node{ - Filesystem: "root", - Path: "/dummy/0", + Path: "/dummy/0", }, }, { Node: igntypes.Node{ - Filesystem: "root", - Path: "/dummy/1", + Path: "/dummy/1", }, }} mcs := []*mcfgv1.MachineConfig{ @@ -321,13 +319,11 @@ func TestUpdatesGeneratedMachineConfig(t *testing.T) { mcp := helpers.NewMachineConfigPool("test-cluster-master", helpers.MasterSelector, nil, "") files := []igntypes.File{{ Node: igntypes.Node{ - Filesystem: "root", - Path: "/dummy/0", + Path: "/dummy/0", }, }, { Node: igntypes.Node{ - Filesystem: "root", - Path: "/dummy/1", + Path: "/dummy/1", }, }} mcs := []*mcfgv1.MachineConfig{ @@ -393,13 +389,11 @@ func TestDoNothing(t *testing.T) { mcp := helpers.NewMachineConfigPool("test-cluster-master", helpers.MasterSelector, nil, "") files := []igntypes.File{{ Node: igntypes.Node{ - Filesystem: "root", - Path: "/dummy/0", + Path: "/dummy/0", }, }, { Node: igntypes.Node{ - Filesystem: "root", - Path: "/dummy/1", + Path: "/dummy/1", }, }} mcs := []*mcfgv1.MachineConfig{ diff --git a/pkg/controller/template/render_test.go b/pkg/controller/template/render_test.go index 046e1cd320..f482d3c46f 100644 --- a/pkg/controller/template/render_test.go +++ b/pkg/controller/template/render_test.go @@ -9,7 +9,7 @@ import ( "path/filepath" "testing" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" configv1 "github.com/openshift/api/config/v1" "k8s.io/client-go/kubernetes/scheme" diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 3743b91e71..50ab12929d 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -19,7 +19,7 @@ import ( "time" imgref "github.com/containers/image/docker/reference" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" "github.com/golang/glog" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" @@ -1364,17 +1364,17 @@ func checkUnits(units []igntypes.Unit) error { for _, u := range units { for j := range u.Dropins { path := filepath.Join(pathSystemd, u.Name+".d", u.Dropins[j].Name) - if err := checkFileContentsAndMode(path, []byte(u.Dropins[j].Contents), defaultFilePermissions); err != nil { + if err := checkFileContentsAndMode(path, []byte(*u.Dropins[j].Contents), defaultFilePermissions); err != nil { return err } } - if u.Contents == "" { + if u.Contents == nil || u.Contents == ctrlcommon.StrToPtr("") { continue } path := filepath.Join(pathSystemd, u.Name) - if u.Mask { + if u.Mask != nil && *u.Mask { link, err := filepath.EvalSymlinks(path) if err != nil { return errors.Wrapf(err, "state validation: error while evaluation symlink for path %q", path) @@ -1383,7 +1383,7 @@ func checkUnits(units []igntypes.Unit) error { return errors.Errorf("state validation: invalid unit masked setting. path: %q; expected: %v; received: %v", path, pathDevNull, link) } } - if err := checkFileContentsAndMode(path, []byte(u.Contents), defaultFilePermissions); err != nil { + if err := checkFileContentsAndMode(path, []byte(*u.Contents), defaultFilePermissions); err != nil { return err } @@ -1405,7 +1405,7 @@ func checkFiles(files []igntypes.File) error { if f.Mode != nil { mode = os.FileMode(*f.Mode) } - contents, err := dataurl.DecodeString(f.Contents.Source) + contents, err := dataurl.DecodeString(*f.Contents.Source) if err != nil { return errors.Wrapf(err, "couldn't parse file %q", f.Path) } diff --git a/pkg/daemon/daemon_test.go b/pkg/daemon/daemon_test.go index 0646c6649a..2a3c62419f 100644 --- a/pkg/daemon/daemon_test.go +++ b/pkg/daemon/daemon_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/vincent-petithory/dataurl" @@ -60,14 +60,15 @@ func TestOverwrittenFile(t *testing.T) { fileMode := int(fi.Mode().Perm()) // validate single file + duString := dataurl.EncodeBytes([]byte("hello world\n")) files := []igntypes.File{ { Node: igntypes.Node{ Path: "fixtures/test1.txt", }, FileEmbedded1: igntypes.FileEmbedded1{ - Contents: igntypes.FileContents{ - Source: dataurl.EncodeBytes([]byte("hello world\n")), + Contents: igntypes.Resource{ + Source: &duString, }, Mode: &fileMode, }, @@ -79,14 +80,15 @@ func TestOverwrittenFile(t *testing.T) { } // validate overwritten file + duString2 := dataurl.EncodeBytes([]byte("hello\n")) files = []igntypes.File{ { Node: igntypes.Node{ Path: "fixtures/test1.txt", }, FileEmbedded1: igntypes.FileEmbedded1{ - Contents: igntypes.FileContents{ - Source: dataurl.EncodeBytes([]byte("hello\n")), + Contents: igntypes.Resource{ + Source: &duString2, }, Mode: &fileMode, }, @@ -96,8 +98,8 @@ func TestOverwrittenFile(t *testing.T) { Path: "fixtures/test1.txt", }, FileEmbedded1: igntypes.FileEmbedded1{ - Contents: igntypes.FileContents{ - Source: dataurl.EncodeBytes([]byte("hello world\n")), + Contents: igntypes.Resource{ + Source: &duString, }, Mode: &fileMode, }, diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 5f0d323317..b3fb0d4ca9 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -17,7 +17,7 @@ import ( "time" "github.com/clarketm/json" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" "github.com/golang/glog" "github.com/google/renameio" errors "github.com/pkg/errors" @@ -466,14 +466,6 @@ func Reconcilable(oldConfig, newConfig *mcfgv1.MachineConfig) (*MachineConfigDif return nil, err } - // Networkd section - - // we don't currently configure the network in place. we can't fix it if - // something changed here. - if !reflect.DeepEqual(oldIgn.Networkd, newIgn.Networkd) { - return nil, errors.New("ignition networkd section contains changes") - } - // Passwd section // we don't currently configure Groups in place. we don't configure Users except @@ -531,7 +523,7 @@ func Reconcilable(oldConfig, newConfig *mcfgv1.MachineConfig) (*MachineConfigDif // Special case files append: if the new config wants us to append, then we // have to force a reprovision since it's not idempotent for _, f := range newIgn.Storage.Files { - if f.Append { + if len(f.Append) > 0 { return nil, fmt.Errorf("ignition file %v includes append", f.Path) } } @@ -1085,7 +1077,7 @@ func (dn *Daemon) writeUnits(units []igntypes.Unit) error { return err } } - if err := writeFileAtomicallyWithDefaults(dpath, []byte(u.Dropins[i].Contents)); err != nil { + if err := writeFileAtomicallyWithDefaults(dpath, []byte(*u.Dropins[i].Contents)); err != nil { return fmt.Errorf("failed to write systemd unit dropin %q: %v", u.Dropins[i].Name, err) } @@ -1096,7 +1088,7 @@ func (dn *Daemon) writeUnits(units []igntypes.Unit) error { // check if the unit is masked. if it is, we write a symlink to // /dev/null and continue - if u.Mask { + if u.Mask != nil && *u.Mask { glog.V(2).Info("Systemd unit masked") if err := os.RemoveAll(fpath); err != nil { return fmt.Errorf("failed to remove unit %q: %v", u.Name, err) @@ -1111,7 +1103,7 @@ func (dn *Daemon) writeUnits(units []igntypes.Unit) error { continue } - if u.Contents != "" { + if u.Contents != nil && u.Contents != ctrlcommon.StrToPtr("") { glog.Infof("Writing systemd unit %q", u.Name) if _, err := os.Stat("/usr" + fpath); err == nil && (operatingSystem == machineConfigDaemonOSRHCOS || operatingSystem == machineConfigDaemonOSFCOS) { @@ -1120,7 +1112,7 @@ func (dn *Daemon) writeUnits(units []igntypes.Unit) error { } } // write the unit to disk - if err := writeFileAtomicallyWithDefaults(fpath, []byte(u.Contents)); err != nil { + if err := writeFileAtomicallyWithDefaults(fpath, []byte(*u.Contents)); err != nil { return fmt.Errorf("failed to write systemd unit %q: %v", u.Name, err) } @@ -1133,14 +1125,7 @@ func (dn *Daemon) writeUnits(units []igntypes.Unit) error { // otherwise the unit is disabled. run disableUnit to ensure the unit is // disabled. even if the unit wasn't previously enabled the result will // be fine as disableUnit is idempotent. - // Note: we have to check for legacy unit.Enable and honor it glog.Infof("Enabling systemd unit %q", u.Name) - if u.Enable { - if err := dn.enableUnit(u); err != nil { - return err - } - glog.V(2).Infof("Enabled systemd unit %q", u.Name) - } if u.Enabled != nil { if *u.Enabled { if err := dn.enableUnit(u); err != nil { @@ -1164,7 +1149,7 @@ func (dn *Daemon) writeFiles(files []igntypes.File) error { for _, file := range files { glog.Infof("Writing file %q", file.Path) - contents, err := dataurl.DecodeString(file.Contents.Source) + contents, err := dataurl.DecodeString(*file.Contents.Source) if err != nil { return err } @@ -1172,15 +1157,10 @@ func (dn *Daemon) writeFiles(files []igntypes.File) error { if file.Mode != nil { mode = os.FileMode(*file.Mode) } - var ( - uid, gid = -1, -1 - ) // set chown if file information is provided - if file.User != nil || file.Group != nil { - uid, gid, err = getFileOwnership(file) - if err != nil { - return fmt.Errorf("failed to retrieve file ownership for file %q: %v", file.Path, err) - } + uid, gid, err := getFileOwnership(file) + if err != nil { + return fmt.Errorf("failed to retrieve file ownership for file %q: %v", file.Path, err) } if err := createOrigFile(file.Path, file.Path); err != nil { return err @@ -1245,29 +1225,25 @@ func createOrigFile(fromPath, fpath string) error { // This is essentially ResolveNodeUidAndGid() from Ignition; XXX should dedupe func getFileOwnership(file igntypes.File) (int, int, error) { uid, gid := 0, 0 // default to root - if file.User != nil { - if file.User.ID != nil { - uid = *file.User.ID - } else if file.User.Name != "" { - osUser, err := user.Lookup(file.User.Name) - if err != nil { - return uid, gid, fmt.Errorf("failed to retrieve UserID for username: %s", file.User.Name) - } - glog.V(2).Infof("Retrieved UserId: %s for username: %s", osUser.Uid, file.User.Name) - uid, _ = strconv.Atoi(osUser.Uid) + if file.User.ID != nil { + uid = *file.User.ID + } else if file.User.Name != nil && file.User.Name != ctrlcommon.StrToPtr("") { + osUser, err := user.Lookup(*file.User.Name) + if err != nil { + return uid, gid, fmt.Errorf("failed to retrieve UserID for username: %v", file.User.Name) } + glog.V(2).Infof("Retrieved UserId: %s for username: %s", osUser.Uid, *file.User.Name) + uid, _ = strconv.Atoi(osUser.Uid) } - if file.Group != nil { - if file.Group.ID != nil { - gid = *file.Group.ID - } else if file.Group.Name != "" { - osGroup, err := user.LookupGroup(file.Group.Name) - if err != nil { - return uid, gid, fmt.Errorf("failed to retrieve GroupID for group: %s", file.Group.Name) - } - glog.V(2).Infof("Retrieved GroupID: %s for group: %s", osGroup.Gid, file.Group.Name) - gid, _ = strconv.Atoi(osGroup.Gid) + if file.Group.ID != nil { + gid = *file.Group.ID + } else if file.Group.Name != nil && file.User.Name != ctrlcommon.StrToPtr("") { + osGroup, err := user.LookupGroup(*file.Group.Name) + if err != nil { + return uid, gid, fmt.Errorf("failed to retrieve GroupID for group: %v", file.Group.Name) } + glog.V(2).Infof("Retrieved GroupID: %s for group: %s", osGroup.Gid, *file.Group.Name) + gid, _ = strconv.Atoi(osGroup.Gid) } return uid, gid, nil } diff --git a/pkg/daemon/update_test.go b/pkg/daemon/update_test.go index d941824ef0..3899d254fe 100644 --- a/pkg/daemon/update_test.go +++ b/pkg/daemon/update_test.go @@ -7,7 +7,7 @@ import ( "testing" "time" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" "github.com/openshift/machine-config-operator/test/helpers" @@ -84,29 +84,9 @@ func TestReconcilable(t *testing.T) { _, isReconcilable = Reconcilable(oldConfig, newConfig) checkReconcilableResults(t, "Ignition", isReconcilable) - // Verify Networkd unit changes react as expected - oldIgnCfg.Networkd = igntypes.Networkd{} - oldConfig = helpers.CreateMachineConfigFromIgnition(oldIgnCfg) - newIgnCfg.Networkd = igntypes.Networkd{ - Units: []igntypes.Networkdunit{ - igntypes.Networkdunit{ - Name: "test.network", - }, - }, - } - newConfig = helpers.CreateMachineConfigFromIgnition(newIgnCfg) - _, isReconcilable = Reconcilable(oldConfig, newConfig) - checkIrreconcilableResults(t, "Networkd", isReconcilable) - - // Match Networkd - oldIgnCfg.Networkd = newIgnCfg.Networkd - oldConfig = helpers.CreateMachineConfigFromIgnition(oldIgnCfg) - _, isReconcilable = Reconcilable(oldConfig, newConfig) - checkReconcilableResults(t, "Networkd", isReconcilable) - // Verify Disk changes react as expected oldIgnCfg.Storage.Disks = []igntypes.Disk{ - igntypes.Disk{ + { Device: "/one", }, } @@ -121,11 +101,11 @@ func TestReconcilable(t *testing.T) { checkReconcilableResults(t, "Disk", isReconcilable) // Verify Filesystems changes react as expected - oldFSPath := "/foo/bar" oldIgnCfg.Storage.Filesystems = []igntypes.Filesystem{ - igntypes.Filesystem{ - Name: "user", - Path: &oldFSPath, + { + Device: "/dev/sda1", + Format: ctrlcommon.StrToPtr("ext4"), + Path: ctrlcommon.StrToPtr("/foo/bar"), }, } oldConfig = helpers.CreateMachineConfigFromIgnition(oldIgnCfg) @@ -140,7 +120,7 @@ func TestReconcilable(t *testing.T) { // Verify Raid changes react as expected oldIgnCfg.Storage.Raid = []igntypes.Raid{ - igntypes.Raid{ + { Name: "data", Level: "stripe", }, @@ -200,8 +180,17 @@ func TestMachineConfigDiff(t *testing.T) { func newTestIgnitionFile(i uint) igntypes.File { mode := 0644 - return igntypes.File{Node: igntypes.Node{Path: fmt.Sprintf("/etc/config%d", i), Filesystem: "root"}, - FileEmbedded1: igntypes.FileEmbedded1{Contents: igntypes.FileContents{Source: fmt.Sprintf("data:,config%d", i)}, Mode: &mode}} + return igntypes.File{ + Node: igntypes.Node{ + Path: fmt.Sprintf("/etc/config%d", i), + }, + FileEmbedded1: igntypes.FileEmbedded1{ + Contents: igntypes.Resource{ + Source: ctrlcommon.StrToPtr(fmt.Sprintf("data:,config%d", i)), + }, + Mode: &mode, + }, + } } func newMachineConfigFromFiles(files []igntypes.File) *mcfgv1.MachineConfig { @@ -357,7 +346,8 @@ func TestReconcilableSSH(t *testing.T) { checkIrreconcilableResults(t, "SSH", errMsg) // check that we cannot make updates if any other Passwd.User field is changed. - tempUser4 := igntypes.PasswdUser{Name: "core", SSHAuthorizedKeys: []igntypes.SSHAuthorizedKey{"5678"}, HomeDir: "somedir"} + homedirString := "somedir" + tempUser4 := igntypes.PasswdUser{Name: "core", SSHAuthorizedKeys: []igntypes.SSHAuthorizedKey{"5678"}, HomeDir: &homedirString} newIgnCfg.Passwd.Users[0] = tempUser4 newMcfg = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, errMsg = Reconcilable(oldMcfg, newMcfg) @@ -433,11 +423,11 @@ func TestInvalidIgnConfig(t *testing.T) { oldMcfg := helpers.CreateMachineConfigFromIgnition(oldIgnConfig) // create file to write that contains an impermissable relative path - tempFileContents := igntypes.FileContents{Source: "data:,hello%20world%0A"} + tempFileContents := igntypes.Resource{Source: ctrlcommon.StrToPtr("data:,hello%20world%0A")} tempMode := 420 newIgnConfig := ctrlcommon.NewIgnConfig() newIgnFile := igntypes.File{ - Node: igntypes.Node{Path: "home/core/test", Filesystem: "root"}, + Node: igntypes.Node{Path: "home/core/test"}, FileEmbedded1: igntypes.FileEmbedded1{Contents: tempFileContents, Mode: &tempMode}, } newIgnConfig.Storage.Files = append(newIgnConfig.Storage.Files, newIgnFile) diff --git a/pkg/server/api.go b/pkg/server/api.go index 74475c3254..66f2a3ff59 100644 --- a/pkg/server/api.go +++ b/pkg/server/api.go @@ -129,11 +129,12 @@ func (sh *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - // the internally saved config is always spec v2.2 - // so translation is only necessary when spec v3.1 is requested + // the internally saved config is always spec v3.1 + // so translation is only necessary when spec v2.2 is requested var serveConf *runtime.RawExtension - if reqConfigVer.Equal(*semver.New("3.1.0")) { - converted3, err := ctrlcommon.ConvertRawExtIgnition2to3(conf) + if reqConfigVer.Equal(*semver.New("2.2.0")) { + glog.Infof("converting config from spec v3.1 to v2.2 for request") + converted2, err := ctrlcommon.ConvertRawExtIgnition3to2(conf) if err != nil { w.Header().Set("Content-Length", "0") w.WriteHeader(http.StatusInternalServerError) @@ -141,7 +142,7 @@ func (sh *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - serveConf = &converted3 + serveConf = &converted2 } else { serveConf = conf } diff --git a/pkg/server/server.go b/pkg/server/server.go index 0dcbfa3ee1..48d503ba2b 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -5,7 +5,7 @@ import ( "net/url" "github.com/clarketm/json" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" "github.com/vincent-petithory/dataurl" "k8s.io/apimachinery/pkg/runtime" @@ -27,11 +27,6 @@ const ( // need this is that on bootstrap + first install we don't have the MCD // running and writing that file. machineConfigContentPath = "/etc/mcs-machine-config-content.json" - - // defaultFileSystem defines the default file system to be - // used for writing the ignition files created by the - // server. - defaultFileSystem = "root" ) // kubeconfigFunc fetches the kubeconfig that needs to be served. @@ -138,12 +133,11 @@ func appendFileToRawIgnition(rawExt *runtime.RawExtension, outPath, contents str fileMode := int(420) file := igntypes.File{ Node: igntypes.Node{ - Filesystem: defaultFileSystem, - Path: outPath, + Path: outPath, }, FileEmbedded1: igntypes.FileEmbedded1{ - Contents: igntypes.FileContents{ - Source: getEncodedContent(contents), + Contents: igntypes.Resource{ + Source: ctrlcommon.StrToPtr(getEncodedContent(contents)), }, Mode: &fileMode, }, diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index f7e2e9a321..542f09d1b1 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -10,7 +10,7 @@ import ( "strings" "testing" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" yaml "github.com/ghodss/yaml" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" @@ -270,7 +270,7 @@ func TestClusterServer(t *testing.T) { } foundEncapsulated = true encapMc := new(mcfgv1.MachineConfig) - contents, err := getDecodedContent(f.Contents.Source) + contents, err := getDecodedContent(*f.Contents.Source) assert.Nil(t, err) err = yaml.Unmarshal([]byte(contents), encapMc) assert.Nil(t, err) @@ -331,7 +331,7 @@ func createUnitMap(units []igntypes.Unit) map[string]igntypes.Unit { func createFileMap(files []igntypes.File) map[string]igntypes.File { m := make(map[string]igntypes.File) for i := range files { - file := path.Join(files[i].Filesystem, files[i].Path) + file := files[i].Path m[file] = files[i] } return m diff --git a/test/e2e/mcd_test.go b/test/e2e/mcd_test.go index f0d8fc3748..3d0111f7a2 100644 --- a/test/e2e/mcd_test.go +++ b/test/e2e/mcd_test.go @@ -60,37 +60,37 @@ func mcLabelForWorkers() map[string]string { return mcLabelForRole("worker") } -func createIgnFile(path, content, fs string, mode int) ign2types.File { - return ign2types.File{ - FileEmbedded1: ign2types.FileEmbedded1{ - Contents: ign2types.FileContents{ - Source: content, +func createIgnFile(path, content string, mode int) ign3types.File { + nodeNameStr := "root" + return ign3types.File{ + FileEmbedded1: ign3types.FileEmbedded1{ + Contents: ign3types.Resource{ + Source: &content, }, Mode: &mode, }, - Node: ign2types.Node{ - Filesystem: fs, - Path: path, - User: &ign2types.NodeUser{ - Name: "root", + Node: ign3types.Node{ + Path: path, + User: ign3types.NodeUser{ + Name: &nodeNameStr, }, }, } } -func createMCToAddFileForRole(name, role, filename, data, fs string) *mcfgv1.MachineConfig { +func createMCToAddFileForRole(name, role, filename, data string) *mcfgv1.MachineConfig { mcadd := createMC(fmt.Sprintf("%s-%s", name, uuid.NewUUID()), role) ignConfig := ctrlcommon.NewIgnConfig() - ignFile := createIgnFile(filename, "data:,"+data, fs, 420) + ignFile := createIgnFile(filename, "data:,"+data, 420) ignConfig.Storage.Files = append(ignConfig.Storage.Files, ignFile) rawIgnConfig := helpers.MarshalOrDie(ignConfig) mcadd.Spec.Config.Raw = rawIgnConfig return mcadd } -func createMCToAddFile(name, filename, data, fs string) *mcfgv1.MachineConfig { - return createMCToAddFileForRole(name, "worker", filename, data, fs) +func createMCToAddFile(name, filename, data string) *mcfgv1.MachineConfig { + return createMCToAddFileForRole(name, "worker", filename, data) } func TestMCDeployed(t *testing.T) { @@ -99,7 +99,7 @@ func TestMCDeployed(t *testing.T) { // TODO: bring this back to 10 for i := 0; i < 3; i++ { startTime := time.Now() - mcadd := createMCToAddFile("add-a-file", fmt.Sprintf("/etc/mytestconf%d", i), "test", "root") + mcadd := createMCToAddFile("add-a-file", fmt.Sprintf("/etc/mytestconf%d", i), "test") // create the dummy MC now _, err := cs.MachineConfigs().Create(context.TODO(), mcadd, metav1.CreateOptions{}) @@ -142,7 +142,7 @@ func mcdForNode(cs *framework.ClientSet, node *corev1.Node) (*corev1.Pod, error) return &mcdList.Items[0], nil } -func TestUpdateSSH(t *testing.T) { +func TestUpdateSSHIgnSpecV2(t *testing.T) { cs := framework.NewClientSet("") // create a dummy MC with an sshKey for user Core @@ -160,8 +160,10 @@ func TestUpdateSSH(t *testing.T) { "abc_test", }, } - ignConfig := ctrlcommon.NewIgnConfig() + + ignConfig := ign2types.Config{} ignConfig.Passwd.Users = append(ignConfig.Passwd.Users, tempUser) + ignConfig.Ignition.Version = "2.2.0" rawIgnConfig := helpers.MarshalOrDie(ignConfig) mcadd.Spec.Config.Raw = rawIgnConfig @@ -190,6 +192,58 @@ func TestUpdateSSH(t *testing.T) { } } +func TestUpdateSSHIgnSpecV3(t *testing.T) { + cs := framework.NewClientSet("") + + // create a dummy MC with an sshKey for user Core + mcName := fmt.Sprintf("99-ign3cfg-worker-%s", uuid.NewUUID()) + mcadd := &mcfgv1.MachineConfig{} + mcadd.ObjectMeta = metav1.ObjectMeta{ + Name: mcName, + Labels: mcLabelForWorkers(), + } + // create a new MC that adds a valid user & ssh key + testIgn3Config := ctrlcommon.NewIgnConfig() + tempUser := ign3types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ign3types.SSHAuthorizedKey{"1234_test_ign3"}} + testIgn3Config.Passwd.Users = append(testIgn3Config.Passwd.Users, tempUser) + testIgn3Config.Ignition.Version = "3.1.0" + mode := 420 + testfiledata := "data:,test-ign3-stuff" + 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) + rawIgnConfig := helpers.MarshalOrDie(testIgn3Config) + mcadd.Spec.Config.Raw = rawIgnConfig + + _, err := cs.MachineConfigs().Create(context.TODO(), mcadd, metav1.CreateOptions{}) + require.Nil(t, err, "failed to create MC") + t.Logf("Created %s", mcadd.Name) + + // grab the latest worker- MC + renderedConfig, err := waitForRenderedConfig(t, cs, "worker", mcadd.Name) + require.Nil(t, err) + err = waitForPoolComplete(t, cs, "worker", renderedConfig) + require.Nil(t, err) + nodes, err := getNodesByRole(cs, "worker") + require.Nil(t, err) + for _, node := range nodes { + assert.Equal(t, node.Annotations[constants.CurrentMachineConfigAnnotationKey], renderedConfig) + assert.Equal(t, node.Annotations[constants.MachineConfigDaemonStateAnnotationKey], constants.MachineConfigDaemonStateDone) + + foundSSH := execCmdOnNode(t, cs, node, "grep", "1234_test_ign3", "/rootfs/home/core/.ssh/authorized_keys") + if !strings.Contains(foundSSH, "1234_test_ign3") { + t.Fatalf("updated ssh keys not found in authorized_keys, got %s", foundSSH) + } + t.Logf("Node %s has SSH key", node.Name) + + foundFile := execCmdOnNode(t, cs, node, "cat", "/rootfs/etc/testfileconfig") + if !strings.Contains(foundFile, "test-ign3-stuff") { + t.Fatalf("updated file doesn't contain expected data, got %s", foundFile) + } + t.Logf("Node %s has file", node.Name) + } +} + func TestKernelArguments(t *testing.T) { cs := framework.NewClientSet("") kargsMC := &mcfgv1.MachineConfig{ @@ -300,7 +354,7 @@ func TestKernelType(t *testing.T) { func TestPoolDegradedOnFailToRender(t *testing.T) { cs := framework.NewClientSet("") - mcadd := createMCToAddFile("add-a-file", "/etc/mytestconfs", "test", "root") + mcadd := createMCToAddFile("add-a-file", "/etc/mytestconfs", "test") ignCfg, err := ctrlcommon.IgnParseWrapper(mcadd.Spec.Config.Raw) require.Nil(t, err, "failed to parse ignition config") ignCfg.Ignition.Version = "" // invalid, won't render @@ -349,15 +403,13 @@ func TestReconcileAfterBadMC(t *testing.T) { cs := framework.NewClientSet("") // create a MC that contains a valid ignition config but is not reconcilable - mcadd := createMCToAddFile("add-a-file", "/etc/mytestconfs", "test", "root") + mcadd := createMCToAddFile("add-a-file", "/etc/mytestconfs", "test") ignCfg, err := ctrlcommon.IgnParseWrapper(mcadd.Spec.Config.Raw) require.Nil(t, err, "failed to parse ignition config") - ignCfg.Networkd = ign2types.Networkd{ - Units: []ign2types.Networkdunit{ - { - Name: "test.network", - Contents: "test contents", - }, + // Verify Disk changes react as expected + ignCfg.Storage.Disks = []ign3types.Disk{ + { + Device: "/one", }, } rawIgnCfg := helpers.MarshalOrDie(ignCfg) @@ -451,7 +503,7 @@ func TestReconcileAfterBadMC(t *testing.T) { func TestDontDeleteRPMFiles(t *testing.T) { cs := framework.NewClientSet("") - mcHostFile := createMCToAddFile("modify-host-file", "/etc/motd", "mco-test", "root") + mcHostFile := createMCToAddFile("modify-host-file", "/etc/motd", "mco-test") workerOldMc := getMcName(t, cs, "worker") @@ -500,7 +552,7 @@ func TestCustomPool(t *testing.T) { createMCP(t, cs, "infra") - infraMC := createMCToAddFileForRole("infra-host-file", "infra", "/etc/mco-custom-pool", "mco-custom-pool", "root") + infraMC := createMCToAddFileForRole("infra-host-file", "infra", "/etc/mco-custom-pool", "mco-custom-pool") _, err := cs.MachineConfigs().Create(context.TODO(), infraMC, metav1.CreateOptions{}) require.Nil(t, err) renderedConfig, err := waitForRenderedConfig(t, cs, "infra", infraMC.Name) @@ -550,7 +602,7 @@ func TestIgn3Cfg(t *testing.T) { testIgn3Config := ign3types.Config{} tempUser := ign3types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ign3types.SSHAuthorizedKey{"1234_test_ign3"}} testIgn3Config.Passwd.Users = append(testIgn3Config.Passwd.Users, tempUser) - testIgn3Config.Ignition.Version = "3.0.0" + testIgn3Config.Ignition.Version = "3.1.0" mode := 420 testfiledata := "data:,test-ign3-stuff" tempFile := ign3types.File{Node: ign3types.Node{Path: "/etc/testfileconfig"}, @@ -565,6 +617,7 @@ func TestIgn3Cfg(t *testing.T) { // grab the latest worker- MC renderedConfig, err := waitForRenderedConfig(t, cs, "worker", mcadd.Name) + require.Nil(t, err) err = waitForPoolComplete(t, cs, "worker", renderedConfig) require.Nil(t, err) diff --git a/test/helpers/helpers.go b/test/helpers/helpers.go index 18ca18de81..d14a7b2ede 100644 --- a/test/helpers/helpers.go +++ b/test/helpers/helpers.go @@ -2,7 +2,7 @@ package helpers import ( "github.com/clarketm/json" - igntypes "github.com/coreos/ignition/config/v2_2/types" + igntypes "github.com/coreos/ignition/v2/config/v3_1/types" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" diff --git a/vendor/github.com/coreos/ign-converter/translate/v23tov30/v23tov30.go b/vendor/github.com/coreos/ign-converter/translate/v23tov30/v23tov30.go index fa49a36e43..9ab04a843d 100644 --- a/vendor/github.com/coreos/ign-converter/translate/v23tov30/v23tov30.go +++ b/vendor/github.com/coreos/ign-converter/translate/v23tov30/v23tov30.go @@ -231,6 +231,37 @@ func translateGroups(groups []old.PasswdGroup) (ret []types.PasswdGroup) { return } +func replaceOrAppendDropin(dropins []types.Dropin, dropin types.Dropin) []types.Dropin { + for i, d := range dropins { + if d.Name == dropin.Name { + dropins[i] = dropin + return dropins + } + } + dropins = append(dropins, dropin) + return dropins +} + +func replaceOrAppendUnit(units []types.Unit, unit types.Unit) []types.Unit { + for i, u := range units { + if u.Name == unit.Name { + // Replace existing contents only if new contents are not empty. + // The unit may define dropins only and have no content, + // in which case we don't want to overwrite the unit contents + // and only append the dropins. + if unit.Contents != nil && unit.Contents != util.StrPStrict("") { + units[i].Contents = unit.Contents + } + for _, dropin := range unit.Dropins { + units[i].Dropins = replaceOrAppendDropin(units[i].Dropins, dropin) + } + return units + } + } + units = append(units, unit) + return units +} + func translateUnits(units []old.Unit) (ret []types.Unit) { for _, u := range units { var enabled *bool @@ -245,7 +276,8 @@ func translateUnits(units []old.Unit) (ret []types.Unit) { if u.Enabled != nil && !*u.Enabled { enabled = util.BoolPStrict(false) } - ret = append(ret, types.Unit{ + // ensure units are deduplicated for spec v3 + ret = replaceOrAppendUnit(ret, types.Unit{ Name: u.Name, Enabled: enabled, Mask: util.BoolP(u.Mask), @@ -370,6 +402,17 @@ func translateNode(n old.Node, m map[string]string) types.Node { } } +func replaceOrAppendFile(files []types.File, file types.File) []types.File { + for i, f := range files { + if f.Node.Path == file.Node.Path { + files[i] = file + return files + } + } + files = append(files, file) + return files +} + func translateFiles(files []old.File, m map[string]string) (ret []types.File) { for _, f := range files { // 2.x files are overwrite by default @@ -400,7 +443,8 @@ func translateFiles(files []old.File, m map[string]string) (ret []types.File) { } else { file.Contents = c } - ret = append(ret, file) + // ensure files are deduplicated for spec v3 + ret = replaceOrAppendFile(ret, file) } return } diff --git a/vendor/modules.txt b/vendor/modules.txt index 42b80e825b..d80c791e08 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -60,7 +60,7 @@ github.com/coreos/go-semver/semver github.com/coreos/go-systemd/unit # github.com/coreos/go-systemd/v22 v22.0.0 github.com/coreos/go-systemd/v22/unit -# github.com/coreos/ign-converter v0.0.0-20200629171308-e40a44f244c5 +# github.com/coreos/ign-converter v0.0.0-20200629171308-e40a44f244c5 => github.com/LorbusChris/ign-converter v0.0.0-20200701232648-56880ed0a25d github.com/coreos/ign-converter/translate/v23tov30 github.com/coreos/ign-converter/translate/v31tov22 github.com/coreos/ign-converter/util