From fd38dd34f945bedb9728139a38573806d73c6b59 Mon Sep 17 00:00:00 2001
From: Herman Slatman <hermanslatman@hotmail.com>
Date: Mon, 24 Oct 2022 14:51:27 +0200
Subject: [PATCH] Fix PR comments

---
 authority/authority.go | 28 ++++++++--------------------
 pki/pki.go             | 40 ++++++++++++++++++++--------------------
 2 files changed, 28 insertions(+), 40 deletions(-)

diff --git a/authority/authority.go b/authority/authority.go
index c3155d96..47a1de1c 100644
--- a/authority/authority.go
+++ b/authority/authority.go
@@ -633,23 +633,11 @@ func (a *Authority) init() error {
 
 				c := a.config
 				if c.WasLoadedFromFile() {
-					// TODO(hs): check if prerequisites for writing files look OK (user/group, permission bits, etc) as
-					// extra safety check before trying to write at all?
-
-					// Remove the existing provisioners from the authority configuration
-					// and commit it to the existing configuration file. NOTE: committing
-					// the configuration at this point also writes other properties that
-					// have been initialized with default values, such as the `backdate` and
-					// `template` settings in the `authority`.
-					oldProvisioners := c.AuthorityConfig.Provisioners
-					c.AuthorityConfig.Provisioners = []provisioner.Interface{}
-					if err := c.Commit(); err != nil {
-						// Restore the provisioners in in-memory representation for consistency
-						// when writing the updated configuration fails. This is considered a soft
-						// error, so execution can continue.
-						c.AuthorityConfig.Provisioners = oldProvisioners
-						a.initLogf("Failed removing provisioners from configuration: %v", err)
-					}
+					// The provisioners in the configuration file can be deleted from
+					// the file by editing it. Automatic rewriting of the file was considered
+					// to be too surprising for users and not the right solution for all
+					// use cases, so we leave it up to users to this themselves.
+					a.initLogf("Provisioners that were migrated can now be removed from `ca.json` by editing it.")
 				}
 
 				a.initLogf("Finished migrating provisioners")
@@ -673,16 +661,16 @@ func (a *Authority) init() error {
 			// case if `step` isn't allowed to be signed by Name Constraints or the X.509 policy.
 			// We have protection for that when creating and updating a policy, but if a policy or
 			// Name Constraints are in use at the time of migration, that could lock the user out.
-			firstSuperAdminSubject := "step"
+			superAdminSubject := "step"
 			if err := a.adminDB.CreateAdmin(ctx, &linkedca.Admin{
 				ProvisionerId: firstJWKProvisioner.Id,
-				Subject:       firstSuperAdminSubject,
+				Subject:       superAdminSubject,
 				Type:          linkedca.Admin_SUPER_ADMIN,
 			}); err != nil {
 				return admin.WrapErrorISE(err, "error creating first admin")
 			}
 
-			a.initLogf("Created super admin %q for JWK provisioner %q", firstSuperAdminSubject, firstJWKProvisioner.GetName())
+			a.initLogf("Created super admin %q for JWK provisioner %q", superAdminSubject, firstJWKProvisioner.GetName())
 		}
 	}
 
diff --git a/pki/pki.go b/pki/pki.go
index df65a721..cee3f06a 100644
--- a/pki/pki.go
+++ b/pki/pki.go
@@ -175,19 +175,19 @@ func GetProvisionerKey(caURL, rootFile, kid string) (string, error) {
 }
 
 type options struct {
-	provisioner            string
-	firstSuperAdminSubject string
-	pkiOnly                bool
-	enableACME             bool
-	enableSSH              bool
-	enableAdmin            bool
-	noDB                   bool
-	isHelm                 bool
-	deploymentType         DeploymentType
-	rootKeyURI             string
-	intermediateKeyURI     string
-	hostKeyURI             string
-	userKeyURI             string
+	provisioner        string
+	superAdminSubject  string
+	pkiOnly            bool
+	enableACME         bool
+	enableSSH          bool
+	enableAdmin        bool
+	noDB               bool
+	isHelm             bool
+	deploymentType     DeploymentType
+	rootKeyURI         string
+	intermediateKeyURI string
+	hostKeyURI         string
+	userKeyURI         string
 }
 
 // Option is the type of a configuration option on the pki constructor.
@@ -221,12 +221,12 @@ func WithProvisioner(s string) Option {
 	}
 }
 
-// WithFirstSuperAdminSubject defines the subject of the first
+// WithSuperAdminSubject defines the subject of the first
 // super admin for use with the Admin API. The admin will belong
 // to the first JWK provisioner.
-func WithFirstSuperAdminSubject(s string) Option {
+func WithSuperAdminSubject(s string) Option {
 	return func(p *PKI) {
-		p.options.firstSuperAdminSubject = s
+		p.options.superAdminSubject = s
 	}
 }
 
@@ -924,13 +924,13 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) {
 				}
 			}
 			// Add the first provisioner as an admin.
-			firstSuperAdminSubject := "step"
-			if p.options.firstSuperAdminSubject != "" {
-				firstSuperAdminSubject = p.options.firstSuperAdminSubject
+			superAdminSubject := "step"
+			if p.options.superAdminSubject != "" {
+				superAdminSubject = p.options.superAdminSubject
 			}
 			if err := adminDB.CreateAdmin(context.Background(), &linkedca.Admin{
 				AuthorityId:   admin.DefaultAuthorityID,
-				Subject:       firstSuperAdminSubject,
+				Subject:       superAdminSubject,
 				Type:          linkedca.Admin_SUPER_ADMIN,
 				ProvisionerId: adminID,
 			}); err != nil {