Respond with data to all writes in PKI engine (#18222)

* Respond with data to all writes in PKI engine

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

* Add changelog

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

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

builtin/logical/pki/backend_test.go

@@ -5914,7 +5914,7 @@ func TestPKI_ListRevokedCerts(t *testing.T) {
 		"allow_subdomains": "true",
 		"max_ttl":          "1h",
 	})
-	requireSuccessNilResponse(t, resp, err, "error setting up pki role")
+	requireSuccessNonNilResponse(t, resp, err, "error setting up pki role")
 
 	resp, err = CBWrite(b, s, "issue/test", map[string]interface{}{
 		"common_name": "test1.test.com",

builtin/logical/pki/crl_test.go

@@ -89,7 +89,7 @@ func TestBackend_CRLConfig(t *testing.T) {
 				"auto_rebuild":              tc.autoRebuild,
 				"auto_rebuild_grace_period": tc.autoRebuildGracePeriod,
 			})
-			requireSuccessNilResponse(t, resp, err)
+			requireSuccessNonNilResponse(t, resp, err)
 
 			resp, err = CBRead(b, s, "config/crl")
 			requireSuccessNonNilResponse(t, resp, err)

builtin/logical/pki/integration_test.go

@@ -286,7 +286,7 @@ func TestIntegration_SetSignedWithBackwardsPemBundles(t *testing.T) {
 		MountPoint: "pki-int/",
 	})
 	require.NoError(t, err, "failed setting up role example")
-	require.Nil(t, resp, "got non-nil response from setting up role example: %#v", resp)
+	require.NotNil(t, resp, "got nil response from setting up role example: %#v", resp)
 
 	// Issue cert
 	resp, err = intBackend.HandleRequest(context.Background(), &logical.Request{

builtin/logical/pki/ocsp_test.go

@@ -40,7 +40,7 @@ func TestOcsp_Disabled(t *testing.T) {
 			resp, err := CBWrite(b, s, "config/crl", map[string]interface{}{
 				"ocsp_disable": "true",
 			})
-			requireSuccessNilResponse(t, resp, err)
+			requireSuccessNonNilResponse(t, resp, err)
 			resp, err = SendOcspRequest(t, b, s, localTT.reqType, testEnv.leafCertIssuer1, testEnv.issuer1, crypto.SHA1)
 			require.NoError(t, err)
 			requireFieldsSetInResp(t, resp, "http_content_type", "http_status_code", "http_raw_body")
@@ -538,7 +538,7 @@ func setupOcspEnvWithCaKeyConfig(t *testing.T, keyType string, caKeyBits int, ca
 			"issuer_ref":         issuerId,
 			"key_type":           keyType,
 		})
-		requireSuccessNilResponse(t, resp, err, "roles/test"+strconv.FormatInt(int64(i), 10))
+		requireSuccessNonNilResponse(t, resp, err, "roles/test"+strconv.FormatInt(int64(i), 10))
 
 		resp, err = CBWrite(b, s, "issue/test"+strconv.FormatInt(int64(i), 10), map[string]interface{}{
 			"common_name": "test.foobar.com",

builtin/logical/pki/path_config_crl.go

@@ -227,7 +227,18 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra
 		}
 	}
 
-	return nil, nil
+	return &logical.Response{
+		Data: map[string]interface{}{
+			"expiry":                    config.Expiry,
+			"disable":                   config.Disable,
+			"ocsp_disable":              config.OcspDisable,
+			"ocsp_expiry":               config.OcspExpiry,
+			"auto_rebuild":              config.AutoRebuild,
+			"auto_rebuild_grace_period": config.AutoRebuildGracePeriod,
+			"enable_delta":              config.EnableDelta,
+			"delta_rebuild_interval":    config.DeltaRebuildInterval,
+		},
+	}, nil
 }
 
 const pathConfigCRLHelpSyn = `

builtin/logical/pki/path_resign_crls_test.go

@@ -426,14 +426,14 @@ func setupResignCrlMounts(t *testing.T, b1 *backend, s1 logical.Storage, b2 *bac
 		"allow_subdomains": "true",
 		"max_ttl":          "1h",
 	})
-	requireSuccessNilResponse(t, resp, err, "error setting up pki role on backend 1")
+	requireSuccessNonNilResponse(t, resp, err, "error setting up pki role on backend 1")
 
 	resp, err = CBWrite(b2, s2, "roles/test", map[string]interface{}{
 		"allowed_domains":  "test.com",
 		"allow_subdomains": "true",
 		"max_ttl":          "1h",
 	})
-	requireSuccessNilResponse(t, resp, err, "error setting up pki role on backend 2")
+	requireSuccessNonNilResponse(t, resp, err, "error setting up pki role on backend 2")
 
 	// Issue and revoke a cert in backend 1
 	resp, err = CBWrite(b1, s1, "issue/test", map[string]interface{}{

builtin/logical/pki/path_roles.go

@@ -745,9 +745,6 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data
 		return nil, err
 	}
 	if warning != "" {
-		if resp == nil {
-			resp = &logical.Response{}
-		}
 		resp.AddWarning(warning)
 	}
 	if resp.IsError() {
@@ -767,7 +764,7 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data
 }
 
 func validateRole(b *backend, entry *roleEntry, ctx context.Context, s logical.Storage) (*logical.Response, error) {
-	var resp *logical.Response
+	resp := &logical.Response{}
 	var err error
 
 	if entry.MaxTTL > 0 && entry.TTL > entry.MaxTTL {
@@ -828,6 +825,7 @@ func validateRole(b *backend, entry *roleEntry, ctx context.Context, s logical.S
 		return nil, errutil.UserError{Err: err.Error()}
 	}
 
+	resp.Data = entry.ToResponseData()
 	return resp, nil
 }
 

builtin/logical/pki/path_tidy.go

@@ -736,7 +736,23 @@ func (b *backend) pathConfigAutoTidyWrite(ctx context.Context, req *logical.Requ
 		return logical.ErrorResponse("Auto-tidy enabled but no tidy operations were requested. Enable at least one tidy operation to be run (tidy_cert_store / tidy_revoked_certs / tidy_revoked_cert_issuer_associations)."), nil
 	}
 
-	return nil, sc.writeAutoTidyConfig(config)
+	if err := sc.writeAutoTidyConfig(config); err != nil {
+		return nil, err
+	}
+
+	return &logical.Response{
+		Data: map[string]interface{}{
+			"enabled":                               config.Enabled,
+			"interval_duration":                     int(config.Interval / time.Second),
+			"tidy_cert_store":                       config.CertStore,
+			"tidy_revoked_certs":                    config.RevokedCerts,
+			"tidy_revoked_cert_issuer_associations": config.IssuerAssocs,
+			"tidy_expired_issuers":                  config.ExpiredIssuers,
+			"safety_buffer":                         int(config.SafetyBuffer / time.Second),
+			"issuer_safety_buffer":                  int(config.IssuerSafetyBuffer / time.Second),
+			"pause_duration":                        config.PauseDuration.String(),
+		},
+	}, nil
 }
 
 func (b *backend) tidyStatusStart(config *tidyConfig) {

builtin/logical/pki/storage_migrations_test.go

@@ -385,7 +385,7 @@ func TestExpectedOpsWork_PreMigration(t *testing.T) {
 		MountPoint: "pki/",
 	})
 	require.NoError(t, err, "error from creating role")
-	require.Nil(t, resp, "got non-nil response object from creating role")
+	require.NotNil(t, resp, "got nil response object from creating role")
 
 	// List roles
 	resp, err = b.HandleRequest(context.Background(), &logical.Request{
@@ -471,7 +471,7 @@ func TestExpectedOpsWork_PreMigration(t *testing.T) {
 		MountPoint: "pki/",
 	})
 	require.NoError(t, err, "error setting CRL config")
-	require.Nil(t, resp, "got non-nil response setting CRL config")
+	require.NotNil(t, resp, "got nil response setting CRL config")
 
 	// Set URL config
 	resp, err = b.HandleRequest(context.Background(), &logical.Request{

changelog/18222.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/pki: Respond with written data to `config/auto-tidy`, `config/crl`, and `roles/:role`.
+```

command/patch_test.go

@@ -44,13 +44,13 @@ func TestPatchCommand_Run(t *testing.T) {
 		{
 			"force_kvs",
 			[]string{"-force", "pki/roles/example"},
-			"Success!",
+			"allow_localhost",
 			0,
 		},
 		{
 			"force_f_kvs",
 			[]string{"-f", "pki/roles/example"},
-			"Success!",
+			"allow_localhost",
 			0,
 		},
 		{
@@ -62,13 +62,13 @@ func TestPatchCommand_Run(t *testing.T) {
 		{
 			"single_value",
 			[]string{"pki/roles/example", "allow_localhost=true"},
-			"Success!",
+			"allow_localhost",
 			0,
 		},
 		{
 			"multi_value",
 			[]string{"pki/roles/example", "allow_localhost=true", "allowed_domains=true"},
-			"Success!",
+			"allow_localhost",
 			0,
 		},
 	}