Add enableAdmin and enableACME to Helm values.yml generation

[hslatman]

This PR is an attempt at making it possible to process the enableAdmin and enableACME options, so that the generated Helm values.yml for step-ca includes the properties corresponding to these options.

It's an attempt, because it turns out to be more complicated for the enableAdmin option. The option is set to true when --remote-management is provided to ca init. When remote management is enabled, provisioners are stored in the database. But when we're writing the Helm configuration, we can't add provisioners to the DB (which does happen for the non-Helm case), because at that time we don't have a database to write to. This results in the default provisioner not being stored in the database.

@maraino, @dopey what do you think about this? I don't think an Init Container solves this, at least not without also adding some additional logic for handling this. I think we could use some kind of auto migration process that reads the provisioners from ca.json, stores them in the database and deletes them from ca.json. The problem with that is that specifically in this case the ca.json is intended to be read-only, complicating this. One other way I thought of is adding the initial provisioners not through the ca.json, but through another parameter. It would conceptually be fairly close to an Init Container, I guess. Noticed there's a postInitHook that could be of use, maybe?

[maraino]

Right now we have the step provisioner, but an initContainer-like functionality looks like a better idea.

certificates/authority/authority.go

Lines 600 to 616 in 9d4bef8

	if len(provs) == 0 && !strings.EqualFold(a.config.AuthorityConfig.DeploymentType, "linked") {
	// Create First Provisioner
	prov, err := CreateFirstProvisioner(ctx, a.adminDB, string(a.password))
	if err != nil {
	return admin.WrapErrorISE(err, "error creating first provisioner")
	}
	// Create first admin
	if err := a.adminDB.CreateAdmin(ctx, &linkedca.Admin{
	ProvisionerId: prov.Id,
	Subject: "step",
	Type: linkedca.Admin_SUPER_ADMIN,
	}); err != nil {
	return admin.WrapErrorISE(err, "error creating first admin")
	}
	}
	}

[hslatman]

At CA startup, a bit of additional logging was added:

badger 2022/10/06 16:58:51 INFO: All 0 tables opened in 0s
2022/10/06 16:58:53 Starting migration of provisioners
2022/10/06 16:58:53 Migrated JWK provisioner "jwk@example.com" with admin permissions
2022/10/06 16:58:53 Migrated ACME provisioner "acme"
2022/10/06 16:58:53 Finished migrating provisioners
2022/10/06 16:58:53 Created super admin "step" for JWK provisioner "jwk@example.com"
2022/10/06 16:58:53 Starting Smallstep CA/0000000-dev (darwin/arm64)
2022/10/06 16:58:53 Documentation: https://u.step.sm/docs/ca
...

I'll see if I can make it respect the --quiet flag too.

[hslatman]

Linting issue seems to be because of smallstep/workflows@825ed94. Unfortunately doesn't show in which files the errors exist: https://github.com/smallstep/certificates/actions/runs/3226727287/jobs/5280607978.

[dopey]

@herman I'll take a look at those

[hslatman]

@dopey your change worked; no huge number of file exists errors anymore 🙂

Still would need your changes from #1104, but that will probably soon be in.

I really like that handle! Too bad it's not mine 😅

[hslatman]

In 5700116 I changed the Helm template output to include a default SSHPOP provisioner. This wasn't happening before, probably due to how some config and initialization is intertwined. I assume having it create this provisioner is the right behavior for the Helm case too.

There are a couple of things that I think can be improved when creating a configuration and initializing a CA to make it more maintainable and easier to reason about. I've left some notes and TODOs to remind myself. The tests for the Helm template should help with that, but we need more for the general PKI initialization.

[dopey]

Suggested change

	// If true, does not output initialization logs
	// If true, do not output initialization logs

To match the comments above.

[maraino]

In my opinion, we shouldn't rewrite the configuration.

[hslatman]

@maraino I've adopted your suggestions and added logging of the token configuration at startup.

[maraino]

lgtm