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

User units #1303

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
3 changes: 3 additions & 0 deletions config/shared/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ var (
ErrInvalidSystemdDropinExt = errors.New("invalid systemd drop-in extension")
ErrNoSystemdExt = errors.New("no systemd unit extension")
ErrInvalidInstantiatedUnit = errors.New("invalid systemd instantiated unit")
ErrInvalidUnitScope = errors.New("unit scope must be system, user or global")
ErrUnitNoUsersDefined = errors.New("when 'user' scope is used you must set at least one user")
ErrUnitUsersDefined = errors.New("'users' should be specified only if scope is 'user'")

// Misc errors
ErrSourceRequired = errors.New("source is required")
Expand Down
9 changes: 9 additions & 0 deletions config/v3_4_experimental/schema/ignition.json
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,15 @@
"enabled": {
"type": ["boolean", "null"]
},
"scope": {
"type": ["string", "null"]
},
"users": {
"type": "array",
"items": {
"type": "string"
}
},
"mask": {
"type": ["boolean", "null"]
},
Expand Down
11 changes: 11 additions & 0 deletions config/v3_4_experimental/translate/translate.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,22 @@ func translateDirectoryEmbedded1(old old_types.DirectoryEmbedded1) (ret types.Di
return
}

func translateUnit(old old_types.Unit) (ret types.Unit) {
tr := translate.NewTranslator()
tr.Translate(&old.Contents, &ret.Contents)
tr.Translate(&old.Dropins, &ret.Dropins)
tr.Translate(&old.Enabled, &ret.Enabled)
tr.Translate(&old.Mask, &ret.Mask)
tr.Translate(&old.Name, &ret.Name)
return
}

func Translate(old old_types.Config) (ret types.Config) {
tr := translate.NewTranslator()
tr.AddCustomTranslator(translateIgnition)
tr.AddCustomTranslator(translateDirectoryEmbedded1)
tr.AddCustomTranslator(translateFileEmbedded1)
tr.AddCustomTranslator(translateUnit)
tr.Translate(&old, &ret)
return
}
14 changes: 9 additions & 5 deletions config/v3_4_experimental/types/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,17 @@ type Timeouts struct {
}

type Unit struct {
Contents *string `json:"contents,omitempty"`
Dropins []Dropin `json:"dropins,omitempty"`
Enabled *bool `json:"enabled,omitempty"`
Mask *bool `json:"mask,omitempty"`
Name string `json:"name"`
Contents *string `json:"contents,omitempty"`
Dropins []Dropin `json:"dropins,omitempty"`
Enabled *bool `json:"enabled,omitempty"`
Mask *bool `json:"mask,omitempty"`
Name string `json:"name"`
Scope *string `json:"scope,omitempty"`
Users []UnitUser `json:"users,omitempty"`
}

type UnitUser string

type Verification struct {
Hash *string `json:"hash,omitempty"`
}
35 changes: 34 additions & 1 deletion config/v3_4_experimental/types/unit.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ import (
)

func (u Unit) Key() string {
return u.Name
if u.Scope != nil {
return *u.Scope + "." + u.Name
} else {
return "system." + u.Name
}
}

func (d Dropin) Key() string {
Expand All @@ -41,10 +45,39 @@ func (u Unit) Validate(c cpath.ContextPath) (r report.Report) {
r.AddOnError(c, err)

r.AddOnWarn(c, validations.ValidateInstallSection(u.Name, util.IsTrue(u.Enabled), util.NilOrEmpty(u.Contents), opts))
r.AddOnError(c.Append("scope"), validateScope(u.Scope))

err = validateUsers(u)
if err != nil && err == errors.ErrUnitUsersDefined {
r.AddOnWarn(c.Append("users"), err)
} else {
r.AddOnError(c.Append("users"), err)
}

return
}

func validateScope(scope *string) error {
if scope == nil {
return nil
}
switch *scope {
case "system", "user", "global":
return nil
default:
return errors.ErrInvalidUnitScope
}
}

func validateUsers(u Unit) error {
if u.Scope != nil && *u.Scope == "user" && len(u.Users) == 0 {
return errors.ErrUnitNoUsersDefined
} else if len(u.Users) > 0 && *u.Scope != "user" {
return errors.ErrUnitUsersDefined
}
return nil
Nemric marked this conversation as resolved.
Show resolved Hide resolved
}

func validateName(name string) error {
switch path.Ext(name) {
case ".service", ".socket", ".device", ".mount", ".automount", ".swap", ".target", ".path", ".timer", ".snapshot", ".slice", ".scope":
Expand Down
2 changes: 2 additions & 0 deletions docs/configuration-v3_4_experimental.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ The Ignition configuration is a JSON document conforming to the following specif
* **_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.
* **_scope_** (string): `system` for a system unit, `global` for a user unit applied to all users, or `user` for a user unit applied to users specified by **_users_**. Default is `system`.
* **_users_** (list of strings): usernames of users to be affected by a user unit.
* **_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
104 changes: 54 additions & 50 deletions internal/exec/stages/files/units.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type Preset struct {
enabled bool
instantiatable bool
instances []string
scope util.UnitScope
}

// warnOnOldSystemdVersion checks the version of Systemd
Expand Down Expand Up @@ -71,35 +72,33 @@ func (s *stage) createUnits(config types.Config) error {
if err != nil {
return err
}
key := fmt.Sprintf("%s-%s", unitName, identifier)
key := fmt.Sprintf("%s.%s-%s", util.GetUnitScope(unit), unitName, identifier)
if _, ok := presets[key]; ok {
presets[key].instances = append(presets[key].instances, instance)
} else {
presets[key] = &Preset{unitName, *unit.Enabled, true, []string{instance}}
presets[key] = &Preset{unitName, *unit.Enabled, true, []string{instance}, util.GetUnitScope(unit)}
}
} else {
key := fmt.Sprintf("%s-%s", unit.Name, identifier)
if _, ok := presets[unit.Name]; !ok {
presets[key] = &Preset{unit.Name, *unit.Enabled, false, []string{}}
key := fmt.Sprintf("%s-%s", unit.Key(), identifier)
if _, ok := presets[key]; !ok {
presets[key] = &Preset{unit.Name, *unit.Enabled, false, []string{}, util.GetUnitScope(unit)}
} else {
return fmt.Errorf("%q key is already present in the presets map", key)
}
}
}
if unit.Mask != nil {
if *unit.Mask { // mask: true
relabelpath := ""
if err := s.Logger.LogOp(
func() error {
var err error
relabelpath, err = s.MaskUnit(unit)
var err error = s.MaskUnit(unit)
return err
},
"masking unit %q", unit.Name,
"masking unit %q for scope %q", unit.Name, string(util.GetUnitScope(unit)),
); err != nil {
return err
}
s.relabel(relabelpath)
Copy link
Contributor

Choose a reason for hiding this comment

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

How are we now handling relabeling of mask symlinks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are already relabeled at write time ... do we need to ensure relabelling when symlink is changed to /dev/null or else ?
In case of an existing unit that is masked, e.g. zincati.service, do we need relabelling ?
We can do that easily since we can now use s.relabel "inefficiently" ^^ : #1324


} else { // mask: false
masked, err := s.IsUnitMasked(unit)
if err != nil {
Expand All @@ -110,7 +109,7 @@ func (s *stage) createUnits(config types.Config) error {
func() error {
return s.UnmaskUnit(unit)
},
"unmasking unit %q", unit.Name,
"unmasking unit %q for scope %q", unit.Name, string(util.GetUnitScope(unit)),
); err != nil {
return err
}
Expand All @@ -120,7 +119,7 @@ func (s *stage) createUnits(config types.Config) error {
}
// if we have presets then create the systemd preset file.
if len(presets) != 0 {
if err := s.createSystemdPresetFile(presets); err != nil {
if err := s.createSystemdPresetFiles(presets); err != nil {
return err
}
}
Expand All @@ -147,12 +146,8 @@ func parseInstanceUnit(unit types.Unit) (string, string, error) {

// createSystemdPresetFile creates the presetfile for enabled/disabled
// systemd units.
func (s *stage) createSystemdPresetFile(presets map[string]*Preset) error {
if err := s.relabelPath(filepath.Join(s.DestDir, util.PresetPath)); err != nil {
return err
}
func (s *stage) createSystemdPresetFiles(presets map[string]*Preset) error {
hasInstanceUnit := false

// sort the units before writing to the systemd presets file to ensure
// the file is written in a consistent order across multiple runs
unitNames := make([]string, 0, len(presets))
Expand All @@ -164,6 +159,9 @@ func (s *stage) createSystemdPresetFile(presets map[string]*Preset) error {
for _, name := range unitNames {
value := presets[name]
unitString := value.unit
if err := s.relabelPath(filepath.Join(s.DestDir, s.SystemdPresetPath(value.scope))); err != nil {
return err
}
if value.instantiatable {
hasInstanceUnit = true
// Let's say we have two instantiated enabled units listed under
Expand All @@ -173,15 +171,15 @@ func (s *stage) createSystemdPresetFile(presets map[string]*Preset) error {
}
if value.enabled {
if err := s.Logger.LogOp(
func() error { return s.EnableUnit(unitString) },
"setting preset to enabled for %q", unitString,
func() error { return s.EnableUnit(unitString, value.scope) },
"setting %q preset to enabled for %q", value.scope, unitString,
); err != nil {
return err
}
} else {
if err := s.Logger.LogOp(
func() error { return s.DisableUnit(unitString) },
"setting preset to disabled for %q", unitString,
func() error { return s.DisableUnit(unitString, value.scope) },
"setting %q preset to disabled for %q", value.scope, unitString,
); err != nil {
return err
}
Expand All @@ -203,55 +201,61 @@ func (s *stage) createSystemdPresetFile(presets map[string]*Preset) error {
// applies to the unit's dropins.
func (s *stage) writeSystemdUnit(unit types.Unit) error {
return s.Logger.LogOp(func() error {
relabeledDropinDir := false
for _, dropin := range unit.Dropins {
if dropin.Contents == nil {
continue
}
f, err := s.FileFromSystemdUnitDropin(unit, dropin)
fetchops, err := s.FilesFromSystemdUnitDropin(unit, dropin)
if err != nil {
s.Logger.Crit("error converting systemd dropin: %v", err)
return err
}
// trim off prefix since this needs to be relative to the sysroot
if !strings.HasPrefix(f.Node.Path, s.DestDir) {
panic(fmt.Sprintf("Dropin path %s isn't under prefix %s", f.Node.Path, s.DestDir))
}
relabelPath := f.Node.Path[len(s.DestDir):]
if err := s.Logger.LogOp(
func() error { return s.PerformFetch(f) },
"writing systemd drop-in %q at %q", dropin.Name, f.Node.Path,
); err != nil {
return err
}
if !relabeledDropinDir {
s.relabel(filepath.Dir(relabelPath))
relabeledDropinDir = true
for _, f := range fetchops {
relabeledDropinDir := false
// trim off prefix since this needs to be relative to the sysroot
if !strings.HasPrefix(f.Node.Path, s.DestDir) {
panic(fmt.Sprintf("Dropin path %s isn't under prefix %s", f.Node.Path, s.DestDir))
}
relabelPath := f.Node.Path[len(s.DestDir):]
if err := s.Logger.LogOp(
func() error { return s.PerformFetch(f) },
"writing systemd drop-in %q at %q", dropin.Name, f.Node.Path,
); err != nil {
return err
}
if !relabeledDropinDir {
s.relabel(filepath.Dir(relabelPath))
relabeledDropinDir = true
Copy link
Contributor

Choose a reason for hiding this comment

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

This assumes FilesFromSystemdUnitDropin returns values in sorted order.

Overall I'm wondering if it'd be better to have a separate preparatory PR that teaches the relabeling infrastructure to avoid redundant relabeling, so the rest of the codebase doesn't need to think about it anymore. These individual checks are annoying to maintain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, that's why I did it on a previous commit : #1303 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, you did. 🙂 Since it's a cleanup that isn't directly related to this PR, and it's not a tiny change (since it'll allow some existing code to be simplified), it'd be better to move that change to a separate PR that can land first.

Copy link
Contributor Author

@Nemric Nemric Mar 4, 2022

Choose a reason for hiding this comment

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

cf #1324

}
}
}

if cutil.NilOrEmpty(unit.Contents) {
return nil
}

f, err := s.FileFromSystemdUnit(unit)
fetchops, err := s.FilesFromSystemdUnit(unit)
if err != nil {
s.Logger.Crit("error converting unit: %v", err)
return err
}
// trim off prefix since this needs to be relative to the sysroot
if !strings.HasPrefix(f.Node.Path, s.DestDir) {
panic(fmt.Sprintf("Unit path %s isn't under prefix %s", f.Node.Path, s.DestDir))
}
relabelPath := f.Node.Path[len(s.DestDir):]
if err := s.Logger.LogOp(
func() error { return s.PerformFetch(f) },
"writing unit %q at %q", unit.Name, f.Node.Path,
); err != nil {
return err

for _, f := range fetchops {
// trim off prefix since this needs to be relative to the sysroot
if !strings.HasPrefix(f.Node.Path, s.DestDir) {
panic(fmt.Sprintf("Unit path %s isn't under prefix %s", f.Node.Path, s.DestDir))
}
relabelPath := f.Node.Path[len(s.DestDir):]
if err := s.Logger.LogOp(
func() error { return s.PerformFetch(f) },
"writing unit %q at %q", unit.Name, f.Node.Path,
); err != nil {
return err
}

s.relabel(relabelPath)
}
s.relabel(relabelPath)

return nil
}, "processing unit %q", unit.Name)
}, "processing unit %q for scope %q", unit.Name, string(util.GetUnitScope(unit)))
}
Loading