osbuild2: support for servers option in org.osbuild.chrony

Add support for the newly added `servers` option in the
`org.osbuild.chrony` osbuild stage [1]. The option allows one to
specify timeservers to be used by chrony, including a subset of
lower-level configuration options per each server.

Implement a custom JSON marshaller method for `ChronyStageOptions` to
ensure that exactly one of 'timeservers' or 'servers' is specified, as
mandated by the stage schema.

Optional values in `ChronyConfigServer` are declared as pointers to
distinguish the case when the value was explicitly set by the user from
the default value when the structure instance is created. All of these
options should be omitted from the JSON, but only when not explicitly
set, not when their value us "0" for int or "false" for bool. Downside
of this approach is that one can not easily use pointer to a basic
type literal in the struct literal. Passing the basic type literal has
to be workarounded using an intermediate variable, which address is
used in the struct literal.

Add unit test cases for the added functionality.

[1] https://github.com/osbuild/osbuild/pull/692

Signed-off-by: Tomas Hozza <thozza@redhat.com>

internal/osbuild2/chrony_stage.go

@@ -1,11 +1,41 @@
 package osbuild2
 
+import (
+	"encoding/json"
+	"fmt"
+)
+
+// Exactly one of 'Timeservers' or 'Servers' must be specified
 type ChronyStageOptions struct {
-	Timeservers []string `json:"timeservers"`
+	Timeservers []string             `json:"timeservers,omitempty"`
+	Servers     []ChronyConfigServer `json:"servers,omitempty"`
 }
 
 func (ChronyStageOptions) isStageOptions() {}
 
+// Use '*ToPtr()' functions from the internal/common package to set the pointer values in literals
+type ChronyConfigServer struct {
+	Hostname string `json:"hostname"`
+	Minpoll  *int   `json:"minpoll,omitempty"`
+	Maxpoll  *int   `json:"maxpoll,omitempty"`
+	Iburst   *bool  `json:"iburst,omitempty"`
+	Prefer   *bool  `json:"prefer,omitempty"`
+}
+
+// Unexported struct for use in ChronyStageOptions's MarshalJSON() to prevent recursion
+type chronyStageOptions struct {
+	Timeservers []string             `json:"timeservers,omitempty"`
+	Servers     []ChronyConfigServer `json:"servers,omitempty"`
+}
+
+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",

internal/osbuild2/chrony_stage_test.go

@@ -1,6 +1,7 @@
 package osbuild2
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -14,3 +15,28 @@ 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)
+		})
+	}
+}

internal/osbuild2/stage_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/google/uuid"
+	"github.com/osbuild/osbuild-composer/internal/common"
 )
 
 func TestStage_UnmarshalJSON(t *testing.T) {
@@ -83,13 +84,41 @@ func TestStage_UnmarshalJSON(t *testing.T) {
 			},
 		},
 		{
-			name: "chrony",
+			name: "chrony-timeservers",
 			fields: fields{
-				Type:    "org.osbuild.chrony",
-				Options: &ChronyStageOptions{},
+				Type: "org.osbuild.chrony",
+				Options: &ChronyStageOptions{
+					Timeservers: []string{
+						"ntp1.example.com",
+						"ntp2.example.com",
+					},
+				},
 			},
 			args: args{
-				data: []byte(`{"type":"org.osbuild.chrony","options":{"timeservers":null}}`),
+				data: []byte(`{"type":"org.osbuild.chrony","options":{"timeservers":["ntp1.example.com","ntp2.example.com"]}}`),
+			},
+		},
+		{
+			name: "chrony-servers",
+			fields: fields{
+				Type: "org.osbuild.chrony",
+				Options: &ChronyStageOptions{
+					Servers: []ChronyConfigServer{
+						{
+							Hostname: "127.0.0.1",
+							Minpoll:  common.IntToPtr(0),
+							Maxpoll:  common.IntToPtr(4),
+							Iburst:   common.BoolToPtr(true),
+							Prefer:   common.BoolToPtr(false),
+						},
+						{
+							Hostname: "ntp.example.com",
+						},
+					},
+				},
+			},
+			args: args{
+				data: []byte(`{"type":"org.osbuild.chrony","options":{"servers":[{"hostname":"127.0.0.1","minpoll":0,"maxpoll":4,"iburst":true,"prefer":false},{"hostname":"ntp.example.com"}]}}`),
 			},
 		},
 		{