Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Prevent users from using "{}" in template names #845

Merged
merged 1 commit into from
Sep 9, 2014

Conversation

jonboulle
Copy link
Contributor

I was experimenting with unit file templating and built a small tool to generate templates and then start services using {1,2,3} syntax. But accidentally somehow my shell escaped the {} making fleetctl to submit an actual unit named @{1,2,3}.service.

This made me have to go through all sorts of hidden keys and places to clean it up. In some places it actually fleet took it as 1,2 and 3 but in some places not. Maybe there should be some checks for this?

@bcwaldon
Copy link
Contributor

bcwaldon commented Sep 4, 2014

So I tried to reproduce this using master and saw the following behavior:

$ cat examples/foo\@.service
[Service]
ExecStartPre=/usr/bin/echo "instance=%i"
ExecStart=/usr/bin/sleep 1000
core@core-01 ~/fleet $ fleetctl start "examples/foo@{1,2,3}.service"
$ fleetctl list-unit-files
UNIT                    HASH    DSTATE          STATE           TMACHINE
foo@{1,2,3}.service     5f4c1d1 launched        inactive        9559d65f.../172.17.8.103

On the node to-which the unit was scheduled, I see this:

Sep 04 00:14:05 core-03 fleetd[1066]: ERROR generator.go:40: Failed fetching current unit states: Unit name foo@{1,2,3}.service is not valid.

The behavior that I suspect you're seeing was fixed in #793. I totally agree fleet should not accept fleet units with invalid names in the first place, but this is difficult to enforce until all requests are going through the API. I've filed that bug here: #867.

@bcwaldon bcwaldon added the bug label Sep 4, 2014
testCases := []struct {
name string
uf *unit.UnitFile
}{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is intentionally not exhaustive; the correct place for that is the unit tests covering these functions. This is just a simple functional test of the common cases we've seen to ensure that fleetctl does the validation.

@bcwaldon
Copy link
Contributor

bcwaldon commented Sep 9, 2014

@jonboulle LGTM

jonboulle added a commit that referenced this pull request Sep 9, 2014
Prevent users from using "{}" in template names
@jonboulle jonboulle merged commit edafef1 into coreos:master Sep 9, 2014
@jonboulle jonboulle deleted the 845 branch September 9, 2014 23:00
@jonboulle jonboulle added this to the v0.8.1 milestone Sep 9, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants