skx / marionette Goto Github PK

if and unless are useful in rules but get repetitive if multiple rules require the same condition:

package {
    ....
    if => equal("${ARCH}", "x86_64")
}
file {
    ....
    if => equal("${ARCH}", "x86_64")
}
file {
    ....
    if => equal("${ARCH}", "x86_64")
}

A nicer implementation might be to include files based on conditionals:

include "x86_64_stuff.txt" if equal("${ARCH}", "x86_64")

This could use the same conditionals that are available for rules.

Another alternative would be to allow wrapping if and unless around rules:

if equal("${ARCH}", "x86_64") {
    package {
        ....
    }

    # ${ARCH} equals "x86_64" and ${VAR} equals "foo"
    if equal("${VAR}", "foo") {
        file {
            ....
        }
    }
}

Right now -verbose just affects the output of the main driver, which executes the rules.

It seems natural to allow plugins to honor this flag too.

• Define a configuration-struct.
• Populate it, appropriately.
• Pass it to modules.
  • Update all the modules that make sense to use -verbose flag.

Just need to fetch images for my use-case.

• https://docs.docker.com/engine/api/sdk/examples/

As noted in a previous comment this would be useful.

Right now we can use exists path/to/file for the if and unless tokens.

We should allow string comparisions, so I'm going to change the syntax to be function-like:

 "if" => "exists( /path/to/file)"
 "if" => "equals( \"${distribution}\", \"Debian\" )"

However the problem there is that the strings are quoted, which is a pain. Single tokens are OK.

The last few commits to master were focused on reducing the code-complexity - specifically those reports on the goreportcard:

• https://goreportcard.com/report/github.com/skx/marionette

The site is down at the moment, but the complexity score of >15 is handled via gocyclo. I can use that to confirm I've fixed things.

In addition to the complexity issues I see that some modules aren't tested at 100%, and some not at all.

• Update the parser-test at least since that is achingly close to 100% coverage.
• Add some simple end-to-end tests of file, edit, etc modules.
  • ideally each of our modules should have at least one test-case.
  • but that can wait.

Easy wins should be easy, for example the lexer can be updated to use a lookup table for the majority of its token-types.

cron seems like an obvious one, but I'm open to ideas/suggestions.

Nobody uses them, it is a complication we don't need for the moment.

A couple of types, and warnings:

• https://goreportcard.com/report/github.com/skx/marionette

We currently have two conditionals:

• exists(/path/to/file)
• equals( one, two )

These are hardcoded in our executor; move them to their own package and allow new conditionals to be registered without touching the main code.

Assume function signature of:

• blah( args []string) bool, error

Add test-cases for all current conditionals.

Instead of having modules:

• apt
• dnf - TODO
• emerge - TODO
• dpkg
• yum - TODO

We should just have a single "package" module, which looks up the back-end to use dynamically.

```let mysqlcnf = cat ${HOME}/.my.cnf

shell { name => "Adding datadir, adding new",
command => "sh edit_mysql.sh",
unless => contains($mysqlcnf, 'Required')
}

The file is there, contains the string Required, and it doesn't work as it should.

Some suggestions for modules which I think would be useful and general enough to compliment existing modules:

Users

User addition, deletion and modification via useradd, userdel and usermod respectively.

Groups

Similar to users, but with groupadd, groupdel and groupmod.

Package Repositories

package already exists to manage packages, but a good compliment to this would be the management of additional repositories such as PPAs under Ubuntu based systems or things like EPEL for CentOS. Adding can be accomplished with add-apt-repository on Debian based systems but I don't know of a nice way of removing except for "manually" deleting the repo files. I have less experience with CentOS based systems but it seems adding and removing would be similarly "manual".

Posting this to provide somewhere to discuss whether these would be useful modules and possible implementation details.

It occurred to me in a recent comment that it might be nice to write something like this:

let blah = shell{ command => "uptime" }

log { message => "${blah.stdout}" }

Assigning blocks to variables is a big change that is going to be fiddly and annoying. That said allowing a module to generate outputs is a little appealing, whether for debugging or real use.

The HTTP-module is another obvious case where we have status-code, return-line, and content, upon successful execution.

So instead of allowing the variable assignment I'll propose a simple, optional, interface which allows a module to set variables in the environment - scoped by the rule-name.

For example:

shell { name => "uptime", command => "uptime" }
log { message => "${uptime.stdout}" }

Here we see shell is executed with a name uptime. That will set:

• ${uptime.stdout}
• ${uptime.stderr}

That's a simple change, and should allow conditional execution in the future.

TLDR:

• Some modules will be updated to store variables in the environment after they're executed.
• These can be used in later rules in the obvious way.
  • shell { name => "user", command => "whoami"}
  • shell { command => "..", if => equal("${user.stdout}", "root" }

Enjoying the project so far, but ran into something and I'm curious if you have any thoughts on how you might tackle conditional shell operations? ie. calling useradd only if the user doesn't exist.

I ended up something like this, but:

1. I had to force shell redirection to get the command to be evaluated through the shell (using || wasn't enough).
2. It always considers itself "changed".

shell {
    name => "Create ${user} user",
    command => "id ${user} >/dev/null || useradd --system --no-create-home ${user}"
}

What I think might work would be something along the lines of:

shell {
    name => "Create ${user} user",
    command => "useradd --system --no-create-home ${user}",
    unless => true("id ${user}")
}

I think adding true and false might be easy enough to support as conditionals, but I'm yet to try it.

This is fine:

let key="Name"
let val="Steve"

But if you add a trailing semi-colon, which habit forces upon me you get an error:

$ cat temp.in
let key="Name"
let val="Steve";
$ ./marionette temp.in
Error:expected '{', got {EOF }

Ideally we'd allow the trailing semi-colon, but if we don't we could have a better error message.

We should support:

let arch=/usr/bin/arch

No matter how I try to escape, I can't do:

shell { name    => "Adding datadir, adding new",
        command => ["sed '17 i This is Line 17' ${HOME}/.my.cnf" ]
}

Our lexing of variables is overly permissive.

The following fails:

$ cat t.in
let foo="bar"
$ ./marionette t.in 
Error:expected '=', got {EOF }

Here the "=" is parsed as part of the variable name, which is a bug.

Right now we support apt and dpkg - CentOS support coming soon - but that will mean users will need to be careful about what they specify:

    apt { name => "bash" }

vs.

    rpm{ name => "bash" }

Assuming we keep the same key-values/parameters it would be neat to say:

    pkg_add{ name => "bash" }

Where pkg_add will be an alias to dpkg, or rpm|yum|dnf, as appropriate.

To be honest we could probably just use one-handler, pkg|package and handle the aliasing in an internal fashion. Users could use apt, dnf, dpkg, etc, if they wished, but it would be hidden by default.

I see the file module won't create the file if it isn't there - any way to tell it to?

The http module allows a HTTP-status code to be matched, which looks odd as a string. Similarly file-permissions for the file module must currently be expressed as a string because we have no support for numeric types.

Additionally the shell module has a shell => "true" option, which could be a boolean. Although there's no obvious other place where that is used yet.

I think we should parse values as either strings or numbers. We don't necessarily need to make any changes to usage the "StringValue" can convert at run-time easily enough.

The conversion could happen in the parser, or later. But the end result would be that:

  "mode" => 0755

Or

"expect" => 200

Would be synonymous with "mode => "0755" or "expect => "200".

Package installation could result in unexpected packages being installed depending on the order of installation. Take php7.4 for example. The package depends on libapache2-mod-php7.4 OR php7.4-fpm OR php7.4-cgi.

If a package definition of the below is used:

package {
    name => "php",
    package => [ "php7.4", "php7.4-fpm" ],
    state => "installed"
}

will effectively result in

apt-get install php7.4
apt-get install php7.4-fpm

with the issue being that the first command will also install libapache2-mod-php7.4. This is different to running apt-get install php7.4 php7.4-fpm as the dependency resolution will know that the package requirements are met without having to pick additional packages.

This can be resolved by a user by changing the order of the packages in the defintion to package => [ "php7.4-fpm", "php7.4" ].

The easy fix is to document this. The alternative is to change the package module to install all required packages in a single command.

IS there any way to execute a prompt or even read varname ?

Given this input:

foo { name => "Steve",
        param1 => "Blah",
        ... }

Pipe the parameters as a JSON object to the binary ~/.marionette/foo. If the exit-code is zero then we can get the output string and process it if it is:

• "changed"
• "unchanged"

Both the file and directory modules contain code for setting the owner/group of a file. Move this to the file-library.

Note that we should also handle Windows here, since we have a build-failure there:

 GOARCH=amd64 GOOS=windows go build .
# github.com/skx/marionette/modules
modules/module_directory.go:102:27: undefined: syscall.Stat_t
modules/module_directory.go:103:27: undefined: syscall.Stat_t
modules/module_directory.go:122:27: undefined: syscall.Stat_t
modules/module_directory.go:123:27: undefined: syscall.Stat_t
modules/module_file.go:120:27: undefined: syscall.Stat_t
modules/module_file.go:121:27: undefined: syscall.Stat_t
modules/module_file.go:141:27: undefined: syscall.Stat_t
modules/module_file.go:142:27: undefined: syscall.Stat_t

I wrote a couple of large modules today which would have been neater if I'd been able to separate the implementatin into include files.

While it might be nice to allow search-paths, even allowing inclusion from the pwd would suffice.

This might be a bigger job:

mysql {
    username => "root",
    password => "secret",
    database => "users",
    sql => "SELECT COUNT(id) FROM wp_users WHERE login LIKE \"steve%\"",
}

Might be a good example of how it could work..

Looking over the various examples it seems obvious that the conditional-keys we support are nothing more than function-calls:

shell {
             ...
             if => equals("foo", "bar" )
             ...
}

So what if we generalized that? What if we allowed:

let a = uppercase( "${username}" )

Or

directory { 
            target => sort( "/foo", "/foo/bar", "foo/baz/bar" ... )
}

Supporting this would be pretty simple if we dropped the use of the token.* types within the parser and executor. We'd still lex into such things, but then define some new ast-nodes:

ast.String{}
ast.Number{}
ast.Boolean{}
ast.Backtick{}

Once that was done we can define functions as:

type Funcall struct {
    Node
    Name string
    Args []Node
    ..
}

At that point we'd have a cleaner implementation:

• We'd no longer have the executor know about token.* values.
• We could handle all node-types easily, and identically.
• And "conditionals" would be updated to use "real" function-calls.

As a final step we could add some built-in functions which are used more generally:

• upper()
• lower()
• min(x,y)
• max(y,z)

I'm not sure what kind of primitives would make sense, but it feels like losing the token-usage in the middle of the code would be a good thing even if there were zero new additions.

Implementation Plan

• Define new AST types, return them from the parser, and use them in the executor.
  • This will result in zero functional changes.
• Update conditional values to be function-calls.
  • This will result in zero functional changes.
  • As if and unless are the only places where function-calls are supported. If their implementation changes nobody will notice.
• Finally define a few new functions which can be used for assignments, values, etc.
  • This will be the first time the new feature is exposed.

It is important to note that I don't want a whole new programming language, nor do I see adding more than 10-20 primitive functions. Too much complexity would spoil things, but we could definitely add trim and similar utility methods which would be useful.

Right now we have adhoc logging triggered by -verbose.

Instead it would be useful to get more configurable and consistent logging:

• Logging from the driver.
  • Showing files loaded.
• Logging from the parser.
  • Showing what objects were created.
• Logging from the decisions of conditional calls
• Logging from the modules themselves.

Probably worth looking at logging-libraries that are available to allow -logging=debug (for parser), -logging=info for the driver and rules. Or similar.

We should use require to specify our dependency.

Do you think it would be possible to provide a local variable to a file about which directory it exists in? I'm not keen on hardcoding paths to files and was hoping to use something relative to the file itself.

Inside parser.go we ioutil.ReadFile(path) and then NewWithEnvironment(contents of file, p.e). I wonder if we could update p.e in that case to refer to the realpath of path, minus the filename and extension.

Something like p.e.Set("dir", get real path) and then unset it afterwards somehow.

Then inside my rules:

file { 
  target => "/foo"
  source => "${dir}/file_relative_to_this_file"
}

WDYT?

As noted in the recent issue #89.

This came from more of a "I wonder what would happen if..." thought.

Given the following circular dependencies:

log {
	name => "log1"
	message => "one"
	require => "log2"
}

log {
	name => "log2"
	message => "two"
	require => "log3"
}

log {
	name => "log3"
	message => "three"
	require => "log1"
}

The following output is given:

2022/01/09 18:42:06 [USER] two
2022/01/09 18:42:06 [USER] one
2022/01/09 18:42:06 [USER] two
2022/01/09 18:42:06 [USER] three

Now I'm not sure what I was expecting necessarily, possibly a never ending loop, possibly nothing at all, but certainly not that.

The file module creates files with the permission value hardcoded to 0755: https://github.com/skx/marionette/blob/master/modules/module_file.go#L78

In the case that no mode is specified for the file, perhaps the default OS permissions should be applied which will take the umask into account?

We support external/binary plugins as a result of #2 , we should create a directory to hold them inline, and save them to disk on usage.

This would allow others to contribute easily.

Probably the important thing is to write a "WRITING MODULES" guide, and reference the example.

Putting together some decent examples, in #66, made me consider how other people might approach problems.

I tend to use wget for downloading binaries, for example. But these days curl is perhaps more common.

It might be nice to support this:

let download=""

let download="/usr/bin/curl" if exists(/usr/bin/curl)
let download="/usr/bin/wget" if exists(/usr/bin/wget)

fail { message => "Can't find download program", if empty(download)}

Of course there's the obvious argument "If you want to use wget just .. install it":

package { name => "wget", state => "installed", name => "wget:package" }

shell { name => "download" , require => "wget:package" }

Still supporting conditionals in rules and includes does mean that assignments are the odd one out ..

So apologies in advance as this is going to be a bit of a thought dump rather than any concrete ideas or suggestions. I'm hoping it may trigger some ideas or thoughts from others.

The Goal

The ability to:

• Define additional repositories and packages in separate include files for reuse. I may want newer versions of Node.js or PHP on a server and desktop, but would only want Spotify on a desktop, for example.
• Remove unecessary calls to apt-get update which seems inefficient.

The Background

This stems from me adding multiple additional repositories which started off as

file {
    name => "repo:foo",
    target => "/etc/apt/sources.list.d/foo.list",
    content => "deb https://...",
}

package {
    name => "install:foo",
    package => "foo",
    state => "installed",
    update => "yes",
    require => "repo:foo",
}

# Repeat file and package rules for other repos and packages

In this example update => "yes" is required as a new repo file has been added and the system needs to know that it provides foo before being able to install foo. However, in the layout above, adding N repositories following by N package rules will mean apt-get update runs N times.

This layout allows for rule separation but runs apt-get update too often.

The next stage was changing this to something like

# Add repo foo
# Add repo bar

package {
    package => [ "foo", "bar" ],
    update => "yes"
}

This limits the update to run once but means that all packages defined in the additional repos have to be installed in the same package rule and apt-get update will still run on subsequent runs even when no repo definitions have been added or changed.

This layout limits running of apt-get update, although it would still run at least once per execution, but doesn't allow for separating package definitions into multiple include files.

Next up was to run apt-get update manually:

# Add repo foo
# Add repo bar

shell {
    command => "apt-get update"
}

# package foo
# package bar

This is similar to the previous iteration in that apt-get update will still always run once but doesn't help with separation and is fragile as it relies on order of execution being the same as order of definition.

The next change was to make the shell rule triggered and notify from each repo addition, but this goes back to N repo additions trigging the shell rule N times on the first run (i.e. when all repos are new). It does however mean that apt-get update is not run at all on further executions unless repos are changed. This doesn't help with separating into multiple include files.

Ideas

As stated at the start, this isn't really a concrete idea or suggestion, but I'm thinking of something along the lines of a trigger group. This would be where a rule can be defined as triggered but rather than a 1:1 mapping between notifies and trigger execution, it would allow for triggers to react to a group of rules and only execute if one or more rules in the group result in a change being made. Conceptually it makes sense but I'm not sure of implementation at this stage.

I'm also not sure how this would work with rules being split across files as there could be some strange dependencies where the group doesn't know when to trigger as later include files may add additional rules to the group.

Simple conditional based on file-presence:

foo { name => "Blah .. ",
        if => "exists /etc/foo" }

Simple conditional based on file-missing:

foo { name => "Blah .. ",
        unless => "exists /etc/foo" }

So two keys if and unless. We'll support only one thing initially:

• exists /path/to/file

In the future perhaps we could allow /path/to/file contains "text", etc.

Since we're doing that a lot.

I have a use case where it would be useful to be able to expand variables within files copied via source (and I suppose source_url as well). This would probably be disabled by default for backwards compatibility but something like

file {
    "source" => "/path/to/file",
    "templated" => true
}

could be the way to handle it.

While variables are expanded in content statements, expansion in files would be useful for larger (> 1 line) content.

I was going to look at doing this myself but I noticed the environment isn't passed to modules. I'm not sure if, design wise, this would be appropriate?

When working on #66 I had to write:

directory {
          target => "${archive}",
          state  => "present",
}
directory {
          target => "${install}/go-${version}",
          state  => "present",
}

This would have been nicer:

directory {
          target => [ "${archive}", "${install}/go-${version}", ]
          state  => "present",
}

To be honest many of our modules should accept arrays rather than just strings, but it was easier to write them in the way that felt natural - so package installation supports arrays, for example, because I do use that a lot. But log, fail, etc, etc, do not because I didn't need them to do so initially.

This fails to parse:

shell{command => "id"}

Specifically because of the lack of space between shell and the opening block-marker ({).

That seems like something we should be able to parse successfully.

Remove input.txt, and create an examples/ directory with some standalone examples.

Once that is done make a new release.

This is because we spawn a separate parser to process include-files.

Given this example file main.in:

include "temp.in"

shell {
    command => "echo ${value}",
}

And this include file, temp.in:

let value="Hello, world"

We'd expect:

$ marionette -verbose main.in
Hello, world
$

However we get nothing.

Solution:

• Don't use a sub-parser. Keep all state in the same object (probably)?

Looking at #72 I see the following code:

let download="/usr/bin/curl" if exists(/usr/bin/curl)
let download="/usr/bin/wget" if exists(/usr/bin/wget)

This is obviously not portable (to windows), but the intent is merely to allow:

shell{ command => "${download} ..

So the key result should be that we can find a binary named curl or wget on the path. We don't need to consider the specific path. So we should instead write:

let download="curl"   if on_path(curl)
let download="wget" if on_path(wget)

i.e. A new conditional that returns "true" if the named binary is somewhere on the users' path. It will take care of windows with .exe/.com/.bat suffixes too.

if I

directory { name => "Create Datadir",
target => "/opt/deep/down",
mode => "0755",
}

it doesn't work. I should be able to create a nested dir.

let SOME_VAR = "123"
log {
	name => "log-one"
	message => "one - ${SOME_VAR}"
	require => "log-two"
}

let SOME_VAR = "abc"
let COMMAND_RESULT = `find /tmp`
log {
	name => "log-two"
	message => "two - ${SOME_VAR}"
	unless => set("${COMMAND_RESULT}")
}

The above will output

2022/01/11 13:21:35 [USER] two - 123
2022/01/11 13:21:35 [USER] one - 123

There are two issues here.

1. People may think that two - abc should be logged by looking at the definition. It's understandable why it doesn't but it's non-obvious. This may be something that's just accepted and documented as a known limitation.

2. The bigger issue is where the execution is conditional. With the removal of backtick execution from conditionals, log-two in the example ran because COMMAND_RESULT is empty at the point of execution, only after is find /tmp run and /tmp is unlikely to be empty. Now this came from the move to the AST based approach but I think backtick execution would need to make a come back.

What are your thoughts on preventing literal includes occurring more than once? Maybe store them in a map at the start of the include process?

This won't work for the backticks support but for plain literals it might be OK.

Use case:

# systemd.in
shell triggers {
  name => "Reload systemctl daemon",
  command => "systemctl daemon-reload"
}

# service_1.in
include "systemd.in"

file {
  target => "/etc/systemd/system/service_1.service",
  notify => "Reload systemctl daemon"
}

# service_2.in
include "systemd.in"

file {
  target => "/etc/systemd/system/service_2.service",
  notify => "Reload systemctl daemon"
}

Currently triggers an error about the Reload systemctl daemon name not being unique.

As noted in the #89, we invoke command literally unless they contain special characters that imply a shell is necessary (<, >, or |).

Add a shell => "true" flag to force this.

(Optionally we could consider the use of boolean types?)