Add command "nomad tls"

[lhaig]

This PR adds the same functionality that is currently available in Consul.

It adds the following commands
'nomad tls ca create '
'nomad tls cert create -server / -client / - cli '
'nomad tls cert -info '

It also adds the docs

[pkazmierczak]

Hey Lance, nice work!

I left some comments but generally looks good, I also did not notice any errors running tests against this or using it.

My general question is, though: where does this feature come from? Was it requested by someone? Are we sure we want it implemented this way? I'd prefer someone with more seniority within the project to weigh in.

Also, where would the target implementation live? Your PR is against main, are we targeting 1.4 for including this? Should it be backported? Would be great to get some more context.

[jrasell]

Nice one @lhaig and thanks for working on this. I have some overall comments which apply generally or in more than one place, along with some inline. I have not currently used the output certificates to run a Nomad cluster due to time constraints.

• The New functions are only ever used within testing. Can we remove these and instead within the test instantiate the struct like we do here?
• A couple of tests rely on t.Fatal and it would be nice if we could use require instead.
• The create commands take an optional argument which determines the output file prefix. Is there a reason why this couldn't be a flag instead, which seems to suit the optional nature better?
• A lot of the help and flag wording seems overly terse, however, this could be personal preference.
• I wonder if the CLI output when writing files can be more descriptive to help users; something like "CA Certificate saved to: <file_path>" rather than "Saved: <file_path>".
• The certificate create command details setting dc but not region, which seems at odds with our Learn Guide
• Each create command has a number of vars instantiated to store the flag values; it might be nice to have these as part of the command like we do for eval delete and others.
• Use of ioutil.ReadFile is deprecated and we should use os.Readfile instead.

[lhaig]

• The create commands take an optional argument which determines the output file prefix. Is there a reason why this couldn't be a flag instead, which seems to suit the optional nature better?

I have removed this as the prefix did nothing in the code.

[lhaig]

• A couple of tests rely on t.Fatal and it would be nice if we could use require instead.

I have changed the test @jrasell but now I am thinking I should change the name of the test from
TestValidateTLSCACreateCommand_noTabs to TestValidateTLSCACreateCommand_hasTabs

This is the new test

func TestValidateTLSCACreateCommand_HasTabs(t *testing.T) {
	t.Parallel()
	ui := cli.NewMockUi()
	cmd := &TLSCACreateCommand{Meta: Meta{Ui: ui}}
	code := cmd.Help()
	require.False(t, strings.ContainsRune(code, '\t'))
}

[tgross]

Hi @lhaig! This is a great start!

[tgross]

Preview link https://nomad-nttruonxz-hashicorp.vercel.app/nomad/docs/commands/tls

[tgross]

LGTM. @jrasell has open comments that I want to make sure he's satisfied about, but we'll merge once he's had a second chance at it on Monday.

[jrasell]

LGTM; thanks @lhaig! I am very excited about using this for automagic Nomad TLS clusters.

[tgross]

Let's get this merged so it can go out in the next release of Nomad (likely 1.4.4).

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.