Update PKI to new Operations framework (hashicorp#15180)

The backend Framework has updated Callbacks (used extensively in PKI) to
become deprecated; Operations takes their place and clarifies forwarding
of requests.

We switch to the new format everywhere, updating some bad assumptions
about forwarding along the way. Anywhere writes are handled (that should
be propagated to all nodes in all clusters), we choose to forward the
request all the way up to the performance primary cluster's primary
node. This holds for issuers/keys, roles, and configs (such as CRL
config, which is globally set for all clusters despite all clusters
having their own separate CRL).

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

builtin/logical/pki/backend.go

@@ -25,22 +25,27 @@ const (
 
 /*
  * PKI requests are a bit special to keep up with the various failure and load issues.
- * The main ca and intermediate requests are always forwarded to the Primary cluster's active
- * node to write and send the key material/config globally across all clusters.
  *
- * CRL/Revocation and Issued certificate apis are handled by the active node within the cluster
- * they originate. Which means if a request comes into a performance secondary cluster the writes
+ * Any requests to write/delete shared data (such as roles, issuers, keys, and configuration)
+ * are always forwarded to the Primary cluster's active node to write and send the key
+ * material/config globally across all clusters. Reads should be handled locally, to give a
+ * sense of where this cluster's replication state is at.
+ *
+ * CRL/Revocation and Fetch Certificate APIs are handled by the active node within the cluster
+ * they originate. This means, if a request comes into a performance secondary cluster, the writes
  * will be forwarded to that cluster's active node and not go all the way up to the performance primary's
  * active node.
  *
- * If a certificate issue request has a role in which no_store is set to true that node itself
- * will issue the certificate and not forward the request to the active node.
+ * If a certificate issue request has a role in which no_store is set to true, that node itself
+ * will issue the certificate and not forward the request to the active node, as this does not
+ * need to write to storage.
  *
- * Following the same pattern if a managed key is involved to sign an issued certificate request
+ * Following the same pattern, if a managed key is involved to sign an issued certificate request
  * and the local node does not have access for some reason to it, the request will be forwarded to
  * the active node within the cluster only.
  *
  * To make sense of what goes where the following bits need to be analyzed within the codebase.
+ *
  * 1. The backend LocalStorage paths determine what storage paths will remain within a
  *    cluster and not be forwarded to a performance primary
  * 2. Within each path's OperationHandler definition, check to see if ForwardPerformanceStandby &

builtin/logical/pki/path_config_ca.go

@@ -18,8 +18,13 @@ secret key and certificate.`,
 			},
 		},
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.UpdateOperation: b.pathImportIssuers,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.UpdateOperation: &framework.PathOperation{
+				Callback: b.pathImportIssuers,
+				// Read more about why these flags are set in backend.go.
+				ForwardPerformanceStandby:   true,
+				ForwardPerformanceSecondary: true,
+			},
 		},
 
 		HelpSynopsis:    pathConfigCAHelpSyn,
@@ -49,9 +54,16 @@ func pathConfigIssuers(b *backend) *framework.Path {
 			},
 		},
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ReadOperation:   b.pathCAIssuersRead,
-			logical.UpdateOperation: b.pathCAIssuersWrite,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathCAIssuersRead,
+			},
+			logical.UpdateOperation: &framework.PathOperation{
+				Callback: b.pathCAIssuersWrite,
+				// Read more about why these flags are set in backend.go.
+				ForwardPerformanceStandby:   true,
+				ForwardPerformanceSecondary: true,
+			},
 		},
 
 		HelpSynopsis:    pathConfigIssuersHelpSyn,

builtin/logical/pki/path_config_crl.go

@@ -32,9 +32,16 @@ valid; defaults to 72 hours`,
 			},
 		},
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ReadOperation:   b.pathCRLRead,
-			logical.UpdateOperation: b.pathCRLWrite,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathCRLRead,
+			},
+			logical.UpdateOperation: &framework.PathOperation{
+				Callback: b.pathCRLWrite,
+				// Read more about why these flags are set in backend.go.
+				ForwardPerformanceStandby:   true,
+				ForwardPerformanceSecondary: true,
+			},
 		},
 
 		HelpSynopsis:    pathConfigCRLHelpSyn,

builtin/logical/pki/path_config_urls.go

@@ -34,9 +34,13 @@ for the OCSP servers attribute. See also RFC 5280 Section 4.2.2.1.`,
 			},
 		},
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.UpdateOperation: b.pathWriteURL,
-			logical.ReadOperation:   b.pathReadURL,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.UpdateOperation: &framework.PathOperation{
+				Callback: b.pathWriteURL,
+			},
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathReadURL,
+			},
 		},
 
 		HelpSynopsis:    pathConfigURLsHelpSyn,

builtin/logical/pki/path_fetch.go

@@ -16,8 +16,10 @@ func pathFetchCA(b *backend) *framework.Path {
 	return &framework.Path{
 		Pattern: `ca(/pem)?`,
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ReadOperation: b.pathFetchRead,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathFetchRead,
+			},
 		},
 
 		HelpSynopsis:    pathFetchHelpSyn,
@@ -30,8 +32,10 @@ func pathFetchCAChain(b *backend) *framework.Path {
 	return &framework.Path{
 		Pattern: `(cert/)?ca_chain`,
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ReadOperation: b.pathFetchRead,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathFetchRead,
+			},
 		},
 
 		HelpSynopsis:    pathFetchHelpSyn,
@@ -44,8 +48,10 @@ func pathFetchCRL(b *backend) *framework.Path {
 	return &framework.Path{
 		Pattern: `crl(/pem)?`,
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ReadOperation: b.pathFetchRead,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathFetchRead,
+			},
 		},
 
 		HelpSynopsis:    pathFetchHelpSyn,
@@ -65,8 +71,10 @@ hyphen-separated octal`,
 			},
 		},
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ReadOperation: b.pathFetchRead,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathFetchRead,
+			},
 		},
 
 		HelpSynopsis:    pathFetchHelpSyn,
@@ -87,8 +95,10 @@ hyphen-separated octal`,
 			},
 		},
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ReadOperation: b.pathFetchRead,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathFetchRead,
+			},
 		},
 
 		HelpSynopsis:    pathFetchHelpSyn,
@@ -101,8 +111,10 @@ func pathFetchCRLViaCertPath(b *backend) *framework.Path {
 	return &framework.Path{
 		Pattern: `cert/crl`,
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ReadOperation: b.pathFetchRead,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathFetchRead,
+			},
 		},
 
 		HelpSynopsis:    pathFetchHelpSyn,
@@ -115,8 +127,10 @@ func pathFetchListCerts(b *backend) *framework.Path {
 	return &framework.Path{
 		Pattern: "certs/?$",
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ListOperation: b.pathFetchCertList,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ListOperation: &framework.PathOperation{
+				Callback: b.pathFetchCertList,
+			},
 		},
 
 		HelpSynopsis:    pathFetchHelpSyn,

builtin/logical/pki/path_fetch_issuers.go

@@ -15,8 +15,10 @@ func pathListIssuers(b *backend) *framework.Path {
 	return &framework.Path{
 		Pattern: "issuers/?$",
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ListOperation: b.pathListIssuersHandler,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ListOperation: &framework.PathOperation{
+				Callback: b.pathListIssuersHandler,
+			},
 		},
 
 		HelpSynopsis:    pathListIssuersHelpSyn,
@@ -91,10 +93,22 @@ intermediate CAs and "permit" only for root CAs.`,
 		Pattern: pattern,
 		Fields:  fields,
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ReadOperation:   b.pathGetIssuer,
-			logical.UpdateOperation: b.pathUpdateIssuer,
-			logical.DeleteOperation: b.pathDeleteIssuer,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathGetIssuer,
+			},
+			logical.UpdateOperation: &framework.PathOperation{
+				Callback: b.pathUpdateIssuer,
+				// Read more about why these flags are set in backend.go.
+				ForwardPerformanceStandby:   true,
+				ForwardPerformanceSecondary: true,
+			},
+			logical.DeleteOperation: &framework.PathOperation{
+				Callback: b.pathDeleteIssuer,
+				// Read more about why these flags are set in backend.go.
+				ForwardPerformanceStandby:   true,
+				ForwardPerformanceSecondary: true,
+			},
 		},
 
 		HelpSynopsis:    pathGetIssuerHelpSyn,
@@ -369,8 +383,10 @@ func buildPathGetIssuerCRL(b *backend, pattern string) *framework.Path {
 		Pattern: pattern,
 		Fields:  fields,
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ReadOperation: b.pathGetIssuerCRL,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathGetIssuerCRL,
+			},
 		},
 
 		HelpSynopsis:    pathGetIssuerCRLHelpSyn,

builtin/logical/pki/path_issue_sign.go

@@ -29,8 +29,10 @@ func buildPathIssue(b *backend, pattern string) *framework.Path {
 	ret := &framework.Path{
 		Pattern: pattern,
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.UpdateOperation: b.metricsWrap("issue", roleRequired, b.pathIssue),
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.UpdateOperation: &framework.PathOperation{
+				Callback: b.metricsWrap("issue", roleRequired, b.pathIssue),
+			},
 		},
 
 		HelpSynopsis:    pathIssueHelpSyn,
@@ -55,8 +57,10 @@ func buildPathSign(b *backend, pattern string) *framework.Path {
 	ret := &framework.Path{
 		Pattern: pattern,
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.UpdateOperation: b.metricsWrap("sign", roleRequired, b.pathSign),
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.UpdateOperation: &framework.PathOperation{
+				Callback: b.metricsWrap("sign", roleRequired, b.pathSign),
+			},
 		},
 
 		HelpSynopsis:    pathSignHelpSyn,
@@ -89,8 +93,10 @@ func buildPathIssuerSignVerbatim(b *backend, pattern string) *framework.Path {
 		Pattern: pattern,
 		Fields:  map[string]*framework.FieldSchema{},
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.UpdateOperation: b.metricsWrap("sign-verbatim", roleOptional, b.pathSignVerbatim),
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.UpdateOperation: &framework.PathOperation{
+				Callback: b.metricsWrap("sign-verbatim", roleOptional, b.pathSignVerbatim),
+			},
 		},
 
 		HelpSynopsis:    pathIssuerSignVerbatimHelpSyn,

builtin/logical/pki/path_manage_issuers.go

@@ -84,8 +84,13 @@ secret-key (optional) and certificates.`,
 			},
 		},
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.UpdateOperation: b.pathImportIssuers,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.UpdateOperation: &framework.PathOperation{
+				Callback: b.pathImportIssuers,
+				// Read more about why these flags are set in backend.go
+				ForwardPerformanceStandby:   true,
+				ForwardPerformanceSecondary: true,
+			},
 		},
 
 		HelpSynopsis:    pathImportIssuersHelpSyn,

builtin/logical/pki/path_revoke.go

@@ -22,8 +22,14 @@ hyphen-separated octal`,
 			},
 		},
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.UpdateOperation: b.metricsWrap("revoke", noRole, b.pathRevokeWrite),
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.UpdateOperation: &framework.PathOperation{
+				Callback: b.metricsWrap("revoke", noRole, b.pathRevokeWrite),
+				// This should never be forwarded. See backend.go for more information.
+				// If this needs to write, the entire request will be forwarded to the
+				// active node of the current performance cluster, but we don't want to
+				// forward invalid revoke requests there.
+			},
 		},
 
 		HelpSynopsis:    pathRevokeHelpSyn,
@@ -35,8 +41,14 @@ func pathRotateCRL(b *backend) *framework.Path {
 	return &framework.Path{
 		Pattern: `crl/rotate`,
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ReadOperation: b.pathRotateCRLRead,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathRotateCRLRead,
+				// See backend.go; we will read a lot of data prior to calling write,
+				// so this request should be forwarded when it is first seen, not
+				// when it is ready to write.
+				ForwardPerformanceStandby: true,
+			},
 		},
 
 		HelpSynopsis:    pathRotateCRLHelpSyn,

builtin/logical/pki/path_roles.go

@@ -18,8 +18,10 @@ func pathListRoles(b *backend) *framework.Path {
 	return &framework.Path{
 		Pattern: "roles/?$",
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ListOperation: b.pathRoleList,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ListOperation: &framework.PathOperation{
+				Callback: b.pathRoleList,
+			},
 		},
 
 		HelpSynopsis:    pathListRolesHelpSyn,
@@ -413,10 +415,22 @@ serviced by this role.`,
 			},
 		},
 
-		Callbacks: map[logical.Operation]framework.OperationFunc{
-			logical.ReadOperation:   b.pathRoleRead,
-			logical.UpdateOperation: b.pathRoleCreate,
-			logical.DeleteOperation: b.pathRoleDelete,
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathRoleRead,
+			},
+			logical.UpdateOperation: &framework.PathOperation{
+				Callback: b.pathRoleCreate,
+				// Read more about why these flags are set in backend.go.
+				ForwardPerformanceStandby:   true,
+				ForwardPerformanceSecondary: true,
+			},
+			logical.DeleteOperation: &framework.PathOperation{
+				Callback: b.pathRoleDelete,
+				// Read more about why these flags are set in backend.go.
+				ForwardPerformanceStandby:   true,
+				ForwardPerformanceSecondary: true,
+			},
 		},
 
 		HelpSynopsis:    pathRoleHelpSyn,