particle-os/debian-forge-composer
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

osbuild: remove Timeservers slice from chrony stage

Browse source 
The org.osbuild.chrony stage was extended to allow additional directives
alongside time servers [1].  The old Timeservers string slice was kept
for backwards compatibility.  Removing support for it in
osbuild-composer makes working with the stage's options simpler.  Using
the new struct slice Servers field and only specifying a Hostname for
each element is equivalent to the old behaviour, so no functionality is
lost.

This simplifies the chrony stage since no validation is required
anymore.
It also simplifies the propagation of configuration options through the
pipeline generation code which doesn't need to check for both types of
stage options.

[1] https://github.com/osbuild/osbuild/pull/692
This commit is contained in:
Achilleas Koutsou 2022-11-02 15:30:35 +01:00 • committed by Christian Kellner
parent 0ba1a5ff73
commit 5d760c48ad
11 changed files with 42 additions and 64 deletions
Download patch file Download diff file Expand all files Collapse all files

6
internal/distro/fedora/images.go
View file

@ -91,9 +91,11 @@ func osCustomizations(
}
if len(ntpServers) > 0 {
osc.NTPServers = ntpServers
for _, server := range ntpServers {
osc.NTPServers = append(osc.NTPServers, osbuild.ChronyConfigServer{Hostname: server})
}
} else if imageConfig.TimeSynchronization != nil {
osc.NTPServers = imageConfig.TimeSynchronization.Timeservers
osc.NTPServers = imageConfig.TimeSynchronization.Servers
}
// Relabel the tree, unless the `NoSElinux` flag is explicitly set to `true`

14
internal/distro/image_config_test.go
View file

@ -20,7 +20,7 @@ func TestImageConfigInheritFrom(t *testing.T) {
distroConfig: &ImageConfig{
Timezone: common.StringToPtr("America/New_York"),
TimeSynchronization: &osbuild.ChronyStageOptions{
Timeservers: []string{"127.0.0.1"},
Servers: []osbuild.ChronyConfigServer{{Hostname: "127.0.0.1"}},
},
Locale: common.StringToPtr("en_US.UTF-8"),
Keyboard: &osbuild.KeymapStageOptions{
@ -123,7 +123,7 @@ func TestImageConfigInheritFrom(t *testing.T) {
distroConfig: &ImageConfig{
Timezone: common.StringToPtr("America/New_York"),
TimeSynchronization: &osbuild.ChronyStageOptions{
Timeservers: []string{"127.0.0.1"},
Servers: []osbuild.ChronyConfigServer{{Hostname: "127.0.0.1"}},
},
Locale: common.StringToPtr("en_US.UTF-8"),
Keyboard: &osbuild.KeymapStageOptions{
@ -137,7 +137,7 @@ func TestImageConfigInheritFrom(t *testing.T) {
expectedConfig: &ImageConfig{
Timezone: common.StringToPtr("America/New_York"),
TimeSynchronization: &osbuild.ChronyStageOptions{
Timeservers: []string{"127.0.0.1"},
Servers: []osbuild.ChronyConfigServer{{Hostname: "127.0.0.1"}},
},
Locale: common.StringToPtr("en_US.UTF-8"),
Keyboard: &osbuild.KeymapStageOptions{
@ -154,7 +154,7 @@ func TestImageConfigInheritFrom(t *testing.T) {
imageConfig: &ImageConfig{
Timezone: common.StringToPtr("America/New_York"),
TimeSynchronization: &osbuild.ChronyStageOptions{
Timeservers: []string{"127.0.0.1"},
Servers: []osbuild.ChronyConfigServer{{Hostname: "127.0.0.1"}},
},
Locale: common.StringToPtr("en_US.UTF-8"),
Keyboard: &osbuild.KeymapStageOptions{
@ -167,7 +167,7 @@ func TestImageConfigInheritFrom(t *testing.T) {
expectedConfig: &ImageConfig{
Timezone: common.StringToPtr("America/New_York"),
TimeSynchronization: &osbuild.ChronyStageOptions{
Timeservers: []string{"127.0.0.1"},
Servers: []osbuild.ChronyConfigServer{{Hostname: "127.0.0.1"}},
},
Locale: common.StringToPtr("en_US.UTF-8"),
Keyboard: &osbuild.KeymapStageOptions{
@ -184,7 +184,7 @@ func TestImageConfigInheritFrom(t *testing.T) {
imageConfig: &ImageConfig{
Timezone: common.StringToPtr("America/New_York"),
TimeSynchronization: &osbuild.ChronyStageOptions{
Timeservers: []string{"127.0.0.1"},
Servers: []osbuild.ChronyConfigServer{{Hostname: "127.0.0.1"}},
},
Locale: common.StringToPtr("en_US.UTF-8"),
Keyboard: &osbuild.KeymapStageOptions{
@ -197,7 +197,7 @@ func TestImageConfigInheritFrom(t *testing.T) {
expectedConfig: &ImageConfig{
Timezone: common.StringToPtr("America/New_York"),
TimeSynchronization: &osbuild.ChronyStageOptions{
Timeservers: []string{"127.0.0.1"},
Servers: []osbuild.ChronyConfigServer{{Hostname: "127.0.0.1"}},
},
Locale: common.StringToPtr("en_US.UTF-8"),
Keyboard: &osbuild.KeymapStageOptions{

6
internal/distro/rhel7/pipelines.go
View file

@ -82,7 +82,11 @@ func osPipeline(t *imageType,
}
if len(ntpServers) > 0 {
p.AddStage(osbuild.NewChronyStage(&osbuild.ChronyStageOptions{Timeservers: ntpServers}))
servers := make([]osbuild.ChronyConfigServer, len(ntpServers))
for idx, server := range ntpServers {
servers[idx] = osbuild.ChronyConfigServer{Hostname: server}
}
p.AddStage(osbuild.NewChronyStage(&osbuild.ChronyStageOptions{Servers: servers}))
} else if imageConfig.TimeSynchronization != nil {
p.AddStage(osbuild.NewChronyStage(imageConfig.TimeSynchronization))
}

2
internal/distro/rhel8/distro.go
View file

@ -1348,7 +1348,7 @@ func newDistro(distroName string) distro.Distro {
defaultGceByosImageConfig := &distro.ImageConfig{
Timezone: common.StringToPtr("UTC"),
TimeSynchronization: &osbuild.ChronyStageOptions{
Timeservers: []string{"metadata.google.internal"},
Servers: []osbuild.ChronyConfigServer{{Hostname: "metadata.google.internal"}},
},
Firewall: &osbuild.FirewallStageOptions{
DefaultZone: "trusted",

6
internal/distro/rhel8/pipelines.go
View file

@ -440,7 +440,11 @@ func osPipeline(t *imageType,
}
if len(ntpServers) > 0 {
p.AddStage(osbuild.NewChronyStage(&osbuild.ChronyStageOptions{Timeservers: ntpServers}))
servers := make([]osbuild.ChronyConfigServer, len(ntpServers))
for idx, server := range ntpServers {
servers[idx] = osbuild.ChronyConfigServer{Hostname: server}
}
p.AddStage(osbuild.NewChronyStage(&osbuild.ChronyStageOptions{Servers: servers}))
} else if imageConfig.TimeSynchronization != nil {
p.AddStage(osbuild.NewChronyStage(imageConfig.TimeSynchronization))
}

2
internal/distro/rhel9/gce.go
View file

@ -57,7 +57,7 @@ var (
defaultGceImageConfig = &distro.ImageConfig{
Timezone: common.StringToPtr("UTC"),
TimeSynchronization: &osbuild.ChronyStageOptions{
Timeservers: []string{"metadata.google.internal"},
Servers: []osbuild.ChronyConfigServer{{Hostname: "metadata.google.internal"}},
},
Firewall: &osbuild.FirewallStageOptions{
DefaultZone: "trusted",

7
internal/distro/rhel9/images.go
View file

@ -87,9 +87,12 @@ func osCustomizations(
}
if len(ntpServers) > 0 {
osc.NTPServers = ntpServers
for _, server := range ntpServers {
osc.NTPServers = append(osc.NTPServers, osbuild.ChronyConfigServer{Hostname: server})
}
} else if imageConfig.TimeSynchronization != nil {
osc.NTPServers = imageConfig.TimeSynchronization.Timeservers
osc.NTPServers = imageConfig.TimeSynchronization.Servers
osc.LeapSecTZ = imageConfig.TimeSynchronization.LeapsecTz
}
// Relabel the tree, unless the `NoSElinux` flag is explicitly set to `true`

6
internal/distro/rhel9/pipelines.go
View file

@ -307,7 +307,11 @@ func osPipeline(t *imageType,
}
if len(ntpServers) > 0 {
p.AddStage(osbuild.NewChronyStage(&osbuild.ChronyStageOptions{Timeservers: ntpServers}))
servers := make([]osbuild.ChronyConfigServer, len(ntpServers))
for idx, server := range ntpServers {
servers[idx] = osbuild.ChronyConfigServer{Hostname: server}
}
p.AddStage(osbuild.NewChronyStage(&osbuild.ChronyStageOptions{Servers: servers}))
} else if imageConfig.TimeSynchronization != nil {
p.AddStage(osbuild.NewChronyStage(imageConfig.TimeSynchronization))
}

9
internal/manifest/os.go
View file

@ -47,7 +47,6 @@ type OSCustomizations struct {
Keyboard *string
Hostname string
Timezone string
NTPServers []string
EnabledServices []string
DisabledServices []string
DefaultTarget string
@ -82,6 +81,8 @@ type OSCustomizations struct {
AuthConfig *osbuild.AuthconfigStageOptions
PwQuality *osbuild.PwqualityConfStageOptions
OpenSCAPConfig *osbuild.OscapRemediationStageOptions
NTPServers []osbuild.ChronyConfigServer
LeapSecTZ *string
Subscription *distro.SubscriptionImageOptions
RHSMConfig map[distro.RHSMSubscriptionStatus]*osbuild.RHSMStageOptions
@ -264,7 +265,11 @@ func (p *OS) serialize() osbuild.Pipeline {
pipeline.AddStage(osbuild.NewTimezoneStage(&osbuild.TimezoneStageOptions{Zone: p.Timezone}))
if len(p.NTPServers) > 0 {
pipeline.AddStage(osbuild.NewChronyStage(&osbuild.ChronyStageOptions{Timeservers: p.NTPServers}))
chronyOptions := &osbuild.ChronyStageOptions{Servers: p.NTPServers}
if p.LeapSecTZ != nil {
chronyOptions.LeapsecTz = p.LeapSecTZ
}
pipeline.AddStage(osbuild.NewChronyStage(chronyOptions))
}
if len(p.Groups) > 0 {

22
internal/osbuild/chrony_stage.go
View file

@ -1,15 +1,8 @@
package osbuild
import (
"encoding/json"
"fmt"
)
// Exactly one of 'Timeservers' or 'Servers' must be specified
type ChronyStageOptions struct {
Timeservers []string `json:"timeservers,omitempty"`
Servers []ChronyConfigServer `json:"servers,omitempty"`
LeapsecTz *string `json:"leapsectz,omitempty"`
Servers []ChronyConfigServer `json:"servers,omitempty"`
LeapsecTz *string `json:"leapsectz,omitempty"`
}
func (ChronyStageOptions) isStageOptions() {}
@ -23,17 +16,6 @@ type ChronyConfigServer struct {
Prefer *bool `json:"prefer,omitempty"`
}
// Unexported alias for use in ChronyStageOptions's MarshalJSON() to prevent recursion
type chronyStageOptions ChronyStageOptions
func (o ChronyStageOptions) MarshalJSON() ([]byte, error) {
if (len(o.Timeservers) != 0 && len(o.Servers) != 0) || (len(o.Timeservers) == 0 && len(o.Servers) == 0) {
return nil, fmt.Errorf("exactly one of 'Timeservers' or 'Servers' must be specified")
}
stageOptions := chronyStageOptions(o)
return json.Marshal(stageOptions)
}
func NewChronyStage(options *ChronyStageOptions) *Stage {
return &Stage{
Type: "org.osbuild.chrony",

26
internal/osbuild/chrony_stage_test.go
View file

@ -1,7 +1,6 @@
package osbuild
import (
"encoding/json"
"testing"
"github.com/stretchr/testify/assert"
@ -15,28 +14,3 @@ func TestNewChronyStage(t *testing.T) {
actualStage := NewChronyStage(&ChronyStageOptions{})
assert.Equal(t, expectedStage, actualStage)
}
func TestChronyStage_MarshalJSON_Invalid(t *testing.T) {
tests := []struct {
name string
options ChronyStageOptions
}{
{
name: "not-timeservers-nor-servers",
options: ChronyStageOptions{},
},
{
name: "timeservers-and-servers",
options: ChronyStageOptions{
Timeservers: []string{"ntp.example.com"},
Servers: []ChronyConfigServer{{Hostname: "ntp2.example.com"}},
},
},
}
for idx, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := json.Marshal(tt.options)
assert.NotNilf(t, err, "json.Marshal() didn't return an error [idx: %d]", idx)
})
}
}