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

Node JSON support #101

Closed
wants to merge 5 commits into from
Closed

Node JSON support #101

wants to merge 5 commits into from

Conversation

sandstrom
Copy link
Contributor

@sandstrom sandstrom commented Aug 20, 2018

In chef nodes can be passed individual JSON snippets, called node JSON. Node JSON is used to customize certain things per node.

Some examples are host names, unique node ids, unique tokens for nodes to 'phone home' and report that they are setup successfully (for load-balancers registration, etc), adding per-node unique software license keys, data to seed the random generator and much more.

This commit adds node-JSON support to stemcell.

jq is like sed for JSON, is written in c and has no runtime dependencies. Packages exist in the official repositories for Debian, Ubuntu, Fedora, openSUSE, Arch and Solaris (https://stedolan.github.io/jq/). The footprint is small, it clocks in at ~50 kb.

@darnaut Hi Davi, I'm eager to hear your thoughts on this!

Closes #93

In chef nodes can be passed individual json snippets, called node json,
to customize certain things per node. For example host names, node ids
and similar.

This commit adds node json support to stemcell.

jq is like sed for json, is written in c and has no runtime
dependencies. Packages exist in the official repositories for Debian,
Ubuntu, Fedora, openSUSE, Arch and Solaris
(https://stedolan.github.io/jq/).
@sandstrom
Copy link
Contributor Author

friendly ping! 😄 @darnaut @ljharb @noggi

Copy link
Contributor

@igor47 igor47 left a comment

Choose a reason for hiding this comment

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

besides the nits here, is further parametrizing chef a desirable property? currently, you can say that what chef does on an instance is a function of it's role, branch, and environment, and these are simple strings.

of the reasons mentioned for adding node_json, the only one that really makes sense to me is software license keys. is that an actual use case you'd like? what is the actual reason that this is needed?

@@ -48,6 +49,7 @@ domain_name='<%= opts['instance_domain_name'] %>'
chef_version='<%= opts['chef_version'] %>'
username='<%= opts.fetch('user', ENV['USER']) %>'
encrypted_data_bag_secret_path='<%= opts['chef_data_bag_secret_path'].to_s %>'
node_json='<%= opts['node_json'] %>'
Copy link
Contributor

Choose a reason for hiding this comment

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

the quoting would break if the node json contains ' characters. this requires a more serious escaping strategy -- a HEREDOC might to the trick.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I've added a commit that solves this.

@@ -181,6 +182,14 @@ def launch(opts={})
end
end

if opts['node_json'].is_a?(Hash)
Copy link
Contributor

Choose a reason for hiding this comment

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

do you expect this to be a hash, or a string?

Copy link
Contributor Author

@sandstrom sandstrom Oct 4, 2018

Choose a reason for hiding this comment

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

Both, but I agree that it wasn't very clear!

The old code would (1) turn a hash into JSON string, (2) allow a string (any string) and (3) if no string was set in step (1) or passed in, it would default to an empty JSON object.

I've added an amend commit which improves this: 8c06c9b

@@ -226,7 +232,9 @@ GIT_SSH=${git_wrapper} git fetch
git reset --hard origin/\${branch}
git clean -fdx

json="{\"role\": \"\${role}\", \"env\": \"\${env}\", \"branch\": \"\${branch}\"}"
base_json="{\"role\": \"\${role}\", \"env\": \"\${env}\", \"branch\": \"\${branch}\"}"
json=\`echo \$node_json \$base_json | jq -s '.[0] + .[1]'\`
Copy link
Contributor

Choose a reason for hiding this comment

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

there's no reason to involve an external binary; you can just construct the variable you want in ruby and pass it, fully constructed, into the template.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I've added a commit which moves this over to 'ruby-land'.

@sandstrom
Copy link
Contributor Author

sandstrom commented Oct 4, 2018

@igor47 First of all, thanks for reviewing! 😄 Much appreciated!

I've addressed your points in new commits. Let me know if you have more comments or suggestions.

Regarding the use-case, I'd say there are a few:

  • License keys
  • Custom hostnames
  • Post setup phone home (will usually require a node-unique identifier)
  • Other custom identifiers, for example in a Consul cluster or for databases in a replication ring/cluster

I realize that everyone may not have these uses, but node-json is part of Chef itself and commonly used. This would also lessen the need for wider cloud-init support (since many of the things that cloud-init solves can also be solved via Chef node-json).

Also, great if you'd like to take a look at #100 too.

@sandstrom
Copy link
Contributor Author

sandstrom commented Oct 8, 2018

@igor47 I've run across an issue related to node-data, but for environment.

On master this tool set the environment via a env key in the custom-json passed to the node.
Code here: https://github.com/airbnb/stemcell/blob/master/lib/stemcell/templates/bootstrap.sh.erb#L229

Is this an old way of setting the env in chef, or something custom? Afaik it should be set via the -E flag passed to chef-solo (docs here).

Also, the runlist/role/env dance, is that something custom?

runlist="role[\${role}]"
if [ -f "roles/\${env}.rb" ]
then
  runlist+=",role[\${env}]"
fi

I've added a commit here that should fix this issue 4a08273, let me know what you think! I can also open a separate PR for that, but it's related to custom json so easier to track here if possible.

@sandstrom
Copy link
Contributor Author

@igor47 friendly ping 😄

@sandstrom
Copy link
Contributor Author

Hey @igor47, do you have a few minutes to look at this?

@sandstrom sandstrom closed this by deleting the head repository Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants