Skip to content
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.

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

*: switch systemd & dropin Contents to Resource type #986

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions config/v3_2_experimental/schema/ignition.json
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@
"type": ["boolean", "null"]
},
"contents": {
"type": ["string", "null"]
"$ref": "#/definitions/resource"
},
"dropins": {
"type": "array",
Expand All @@ -441,7 +441,7 @@
"type": "string"
},
"contents": {
"type": ["string", "null"]
"$ref": "#/definitions/resource"
}
},
"required": [
Expand Down
6 changes: 3 additions & 3 deletions config/v3_2_experimental/types/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ type Disk struct {
}

type Dropin struct {
Contents *string `json:"contents,omitempty"`
Name string `json:"name"`
Contents Resource `json:"contents,omitempty"`
Name string `json:"name"`
}

type File struct {
Expand Down Expand Up @@ -199,7 +199,7 @@ type Timeouts struct {
}

type Unit struct {
Contents *string `json:"contents,omitempty"`
Contents Resource `json:"contents,omitempty"`
Dropins []Dropin `json:"dropins,omitempty"`
Enabled *bool `json:"enabled,omitempty"`
Mask *bool `json:"mask,omitempty"`
Expand Down
25 changes: 2 additions & 23 deletions config/v3_2_experimental/types/unit.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,10 @@
package types

import (
"fmt"
"path"
"strings"

"github.com/coreos/ignition/v2/config/shared/errors"
"github.com/coreos/ignition/v2/config/shared/validations"

"github.com/coreos/go-systemd/v22/unit"
cpath "github.com/coreos/vcontext/path"
"github.com/coreos/vcontext/report"
)
Expand All @@ -36,14 +32,12 @@ func (d Dropin) Key() string {
}

func (u Unit) Validate(c cpath.ContextPath) (r report.Report) {
var err error
r.AddOnError(c.Append("name"), validateName(u.Name))
r.Merge(u.Contents.Validate(c))
c = c.Append("contents")
opts, err := validateUnitContent(u.Contents)
r.AddOnError(c, err)

isEnabled := u.Enabled != nil && *u.Enabled
r.AddOnWarn(c, validations.ValidateInstallSection(u.Name, isEnabled, (u.Contents == nil || *u.Contents == ""), opts))

return
}

Expand All @@ -57,9 +51,6 @@ func validateName(name string) error {
}

func (d Dropin) Validate(c cpath.ContextPath) (r report.Report) {
_, err := validateUnitContent(d.Contents)
r.AddOnError(c.Append("contents"), err)

switch path.Ext(d.Name) {
case ".conf":
default:
Expand All @@ -68,15 +59,3 @@ func (d Dropin) Validate(c cpath.ContextPath) (r report.Report) {

return
}

func validateUnitContent(content *string) ([]*unit.UnitOption, error) {
if content == nil {
return []*unit.UnitOption{}, nil
}
c := strings.NewReader(*content)
opts, err := unit.Deserialize(c)
if err != nil {
return nil, fmt.Errorf("invalid unit content: %s", err)
}
return opts, nil
}
26 changes: 14 additions & 12 deletions config/v3_2_experimental/types/unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package types

import (
"fmt"
"reflect"
"testing"

Expand All @@ -24,6 +23,7 @@ import (

"github.com/coreos/vcontext/path"
"github.com/coreos/vcontext/report"
"github.com/vincent-petithory/dataurl"
)

func TestSystemdUnitValidateContents(t *testing.T) {
Expand All @@ -32,15 +32,16 @@ func TestSystemdUnitValidateContents(t *testing.T) {
out error
}{
{
Unit{Name: "test.service", Contents: util.StrToPtr("[Foo]\nQux=Bar")},
Unit{
Name: "test.service",
Contents: Resource{
Source: util.StrToPtr(dataurl.EncodeBytes([]byte("[Foo]\nQux=Bar"))),
},
},
nil,
},
{
Unit{Name: "test.service", Contents: util.StrToPtr("[Foo")},
fmt.Errorf("invalid unit content: unable to find end of section"),
},
{
Unit{Name: "test.service", Contents: util.StrToPtr(""), Dropins: []Dropin{{}}},
Unit{Name: "test.service", Dropins: []Dropin{{}}},
nil,
},
}
Expand Down Expand Up @@ -88,13 +89,14 @@ func TestSystemdUnitDropInValidate(t *testing.T) {
out error
}{
{
Dropin{Name: "test.conf", Contents: util.StrToPtr("[Foo]\nQux=Bar")},
Dropin{
Name: "test.conf",
Contents: Resource{
Source: util.StrToPtr(dataurl.EncodeBytes([]byte("[Foo]\nQux=Bar"))),
},
},
nil,
},
{
Dropin{Name: "test.conf", Contents: util.StrToPtr("[Foo")},
fmt.Errorf("invalid unit content: unable to find end of section"),
},
}

for i, test := range tests {
Expand Down
14 changes: 14 additions & 0 deletions doc/configuration-v3_2_experimental.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,23 @@ The Ignition configuration is a JSON document conforming to the following specif
* **_enabled_** (boolean): whether or not the service shall be enabled. When true, the service is enabled. When false, the service is disabled. When omitted, the service is unmodified. In order for this to have any effect, the unit must have an install section.
* **_mask_** (boolean): whether or not the service shall be masked. When true, the service is masked by symlinking it to `/dev/null`.
* **_contents_** (string): the contents of the unit.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/string/object/ I think? Also, isn't this a major schema break?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, yeah. This would require us to move to spec 4.0.0. Arguably that's okay -- the change is still translatable, so we can support both 3.x and 4.0 in the same code -- but we've avoided incompatible changes so far. But it also makes the original use case harder to implement in handwritten configs, since they'd need to convert to data URLs.

* **_compression_** (string): the type of compression used on the contents (null or gzip). Compression cannot be used with S3.
* **_source_** (string): the URL of the file contents. Supported schemes are `http`, `https`, `tftp`, `s3`, and [`data`][rfc2397]. When using `http`, it is advisable to use the verification option to ensure the contents haven't been modified. If source is omitted and a regular file already exists at the path, Ignition will do nothing. If source is omitted and no file exists, an empty file will be created.
* **_httpHeaders_** (list of objects): a list of HTTP headers to be added to the request. Available for `http` and `https` source schemes only.
* **name** (string): the header name.
* **_value_** (string): the header contents.
* **_verification_** (object): options related to the verification of the file contents.
* **_hash_** (string): the hash of the contents, in the form `<type>-<value>` where type is either `sha512` or `sha256`.
* **_dropins_** (list of objects): the list of drop-ins for the unit. Every drop-in must have a unique `name`.
* **name** (string): the name of the drop-in. This must be suffixed with ".conf".
* **_contents_** (string): the contents of the drop-in.
* **_compression_** (string): the type of compression used on the contents (null or gzip). Compression cannot be used with S3.
* **_source_** (string): the URL of the file contents. Supported schemes are `http`, `https`, `tftp`, `s3`, and [`data`][rfc2397]. When using `http`, it is advisable to use the verification option to ensure the contents haven't been modified. If source is omitted and a regular file already exists at the path, Ignition will do nothing. If source is omitted and no file exists, an empty file will be created.
* **_httpHeaders_** (list of objects): a list of HTTP headers to be added to the request. Available for `http` and `https` source schemes only.
* **name** (string): the header name.
* **_value_** (string): the header contents.
* **_verification_** (object): options related to the verification of the file contents.
* **_hash_** (string): the hash of the contents, in the form `<type>-<value>` where type is either `sha512` or `sha256`.
* **_passwd_** (object): describes the desired additions to the passwd database.
* **_users_** (list of objects): the list of accounts that shall exist. All users must have a unique `name`.
* **name** (string): the username for the account.
Expand Down
10 changes: 7 additions & 3 deletions doc/examples.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ This config will write a single service unit (shown below) with the contents of
"units": [{
"name": "example.service",
"enabled": true,
"contents": "[Service]\nType=oneshot\nExecStart=/usr/bin/echo Hello World\n\n[Install]\nWantedBy=multi-user.target"
"contents": {
"source": "data:text/plain;charset=utf-8;base64,W1NlcnZpY2VdClR5cGU9b25lc2hvdApFeGVjU3RhcnQ9L3Vzci9iaW4vZWNobyBIZWxsbyBXb3JsZAoKW0luc3RhbGxdCldhbnRlZEJ5PW11bHRpLXVzZXIudGFyZ2V0"
}
}]
}
}
Expand Down Expand Up @@ -44,7 +46,9 @@ This config will add a [systemd unit drop-in](https://coreos.com/os/docs/latest/
"name": "systemd-journald.service",
"dropins": [{
"name": "debug.conf",
"contents": "[Service]\nEnvironment=SYSTEMD_LOG_LEVEL=debug"
"contents": {
"source": "data:text/plain;charset=utf-8;base64,W1NlcnZpY2VdCkVudmlyb25tZW50PVNZU1RFTURfTE9HX0xFVkVMPWRlYnVn"
}
}]
}]
}
Expand Down Expand Up @@ -157,7 +161,7 @@ In many scenarios, it may be useful to have an external data volume. This config
"units": [{
"name": "var-lib-data.mount",
"enabled": true,
"contents": "[Mount]\nWhat=/dev/md/data\nWhere=/var/lib/data\nType=ext4\n\n[Install]\nWantedBy=local-fs.target"
"contents": { "source": "data:text/plain;charset=utf-8;base64,W01vdW50XQpXaGF0PS9kZXYvbWQvZGF0YQpXaGVyZT0vdmFyL2xpYi9kYXRhClR5cGU9ZXh0NAoKW0luc3RhbGxdCldhbnRlZEJ5PWxvY2FsLWZzLnRhcmdldA==" }
}]
}
}
Expand Down
5 changes: 3 additions & 2 deletions internal/exec/stages/files/units.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"strings"

"github.com/coreos/ignition/v2/config/shared/errors"
cUtil "github.com/coreos/ignition/v2/config/util"
"github.com/coreos/ignition/v2/config/v3_2_experimental/types"
"github.com/coreos/ignition/v2/internal/distro"
"github.com/coreos/ignition/v2/internal/exec/util"
Expand Down Expand Up @@ -183,7 +184,7 @@ func (s *stage) writeSystemdUnit(unit types.Unit, runtime bool) error {
return s.Logger.LogOp(func() error {
relabeledDropinDir := false
for _, dropin := range unit.Dropins {
if dropin.Contents == nil || *dropin.Contents == "" {
if cUtil.NilOrEmpty(dropin.Contents.Source) {
continue
}
f, err := u.FileFromSystemdUnitDropin(unit, dropin, runtime)
Expand Down Expand Up @@ -211,7 +212,7 @@ func (s *stage) writeSystemdUnit(unit types.Unit, runtime bool) error {
}
}

if unit.Contents == nil || *unit.Contents == "" {
if cUtil.NilOrEmpty(unit.Contents.Source) {
return nil
}

Expand Down
70 changes: 44 additions & 26 deletions internal/exec/util/unit.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,62 +15,85 @@
package util

import (
"encoding/hex"
"fmt"
"net/url"
"os"
"path/filepath"

"github.com/coreos/ignition/v2/config/v3_2_experimental/types"
"github.com/coreos/ignition/v2/internal/distro"

"github.com/vincent-petithory/dataurl"
"github.com/coreos/ignition/v2/internal/resource"
"github.com/coreos/ignition/v2/internal/util"
)

const (
PresetPath string = "/etc/systemd/system-preset/20-ignition.preset"
DefaultPresetPermissions os.FileMode = 0644
)

func (ut Util) FileFromSystemdUnit(unit types.Unit, runtime bool) (FetchOp, error) {
if unit.Contents == nil {
empty := ""
unit.Contents = &empty
func (ut Util) getUnitFetch(path string, contents types.Resource) (FetchOp, error) {
u, err := url.Parse(*contents.Source)
if err != nil {
ut.Logger.Crit("Unable to parse systemd contents URL: %s", err)
return FetchOp{}, err
}
u, err := url.Parse(dataurl.EncodeBytes([]byte(*unit.Contents)))
hasher, err := util.GetHasher(contents.Verification)
if err != nil {
ut.Logger.Crit("Unable to get hasher: %s", err)
return FetchOp{}, err
}

var path string
if runtime {
path = SystemdRuntimeUnitsPath()
} else {
path = SystemdUnitsPath()
var expectedSum []byte
if hasher != nil {
// explicitly ignoring the error here because the config should already
// be validated by this point
_, expectedSumString, _ := util.HashParts(contents.Verification)
expectedSum, err = hex.DecodeString(expectedSumString)
if err != nil {
ut.Logger.Crit("Error parsing verification string %q: %v", expectedSumString, err)
return FetchOp{}, err
}
}

if path, err = ut.JoinPath(path, unit.Name); err != nil {
return FetchOp{}, err
var compression string
if contents.Compression != nil {
compression = *contents.Compression
}

return FetchOp{
Hash: hasher,
Node: types.Node{
Path: path,
},
Url: *u,
FetchOptions: resource.FetchOptions{
Hash: hasher,
ExpectedSum: expectedSum,
Compression: compression,
},
}, nil
}

func (ut Util) FileFromSystemdUnitDropin(unit types.Unit, dropin types.Dropin, runtime bool) (FetchOp, error) {
if dropin.Contents == nil {
empty := ""
dropin.Contents = &empty
func (ut Util) FileFromSystemdUnit(unit types.Unit, runtime bool) (FetchOp, error) {
var path string
var err error
if runtime {
path = SystemdRuntimeUnitsPath()
} else {
path = SystemdUnitsPath()
}
u, err := url.Parse(dataurl.EncodeBytes([]byte(*dropin.Contents)))
if err != nil {

if path, err = ut.JoinPath(path, unit.Name); err != nil {
return FetchOp{}, err
}

return ut.getUnitFetch(path, unit.Contents)
}

func (ut Util) FileFromSystemdUnitDropin(unit types.Unit, dropin types.Dropin, runtime bool) (FetchOp, error) {
var path string
var err error
if runtime {
path = SystemdRuntimeDropinsPath(string(unit.Name))
} else {
Expand All @@ -81,12 +104,7 @@ func (ut Util) FileFromSystemdUnitDropin(unit types.Unit, dropin types.Dropin, r
return FetchOp{}, err
}

return FetchOp{
Node: types.Node{
Path: path,
},
Url: *u,
}, nil
return ut.getUnitFetch(path, unit.Contents)
}

// MaskUnit writes a symlink to /dev/null to mask the specified unit and returns the path of that unit
Expand Down