Adding Response Structures to PKI Config #18376

AnPucel · 2022-12-14T19:56:06Z

Adding Response Structures to PKI Config. Corresponding response structures are linked below each file name

Please see this RFC for implementation details.

Path	Tested
`config/ca`	yes
`config/issuers`	yes
`root/replace`	none
`config/keys`	none
`config/cluster`	yes
`config/crl`	yes
`config/urls`	yes

Related PRs:

builtin/logical/pki/path_config_urls.go

builtin/logical/pki/path_config_ca.go

dhuckins · 2022-12-15T16:58:08Z

@AnPucel can you provide a link (either to docs or source code) to what you're using as references for the fields?

AnPucel · 2022-12-15T17:49:52Z

@AnPucel can you provide a link (either to docs or source code) to what you're using as references for the fields?

I mostly followed the callback and looked at the structure of the response that was being returned there. So, it's a little tedious, but they're usually in the same file.

averche

This looks reasonable to me, though I only spot checked some fields.

builtin/logical/pki/path_config_urls.go

builtin/logical/pki/path_config_crl.go

dhuckins · 2022-12-15T16:54:29Z

builtin/logical/pki/path_config_crl.go

+						Description: "OK",
+						Fields: map[string]*framework.FieldSchema{
+							"expiry": {
+								Type: framework.TypeString,


[note]
lots of these fields are durations, maybe we can add a format field to framework.FieldSchema eventually to make it more explicit
https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.1.md#data-types
https://pkg.go.dev/github.com/go-openapi/strfmt#section-readme

We have TypeDurationSecond, but I don't know if the response is formatted correctly for that always?

Sometimes there are also string time.Time value when its a concrete date instead of a duration though.

Agreed, since we have the tests now, would it be possible to try it out with TypeTime and TypeDurationSecond?

vault/sdk/framework/openapi.go

Lines 805 to 807 in 40f599b

case TypeTime:

ret.baseType = "string"

ret.format = "date-time"

vault/sdk/framework/openapi.go

Lines 785 to 787 in 40f599b

case TypeDurationSecond, TypeSignedDurationSecond:

ret.baseType = "integer"

ret.format = "seconds"

Same comment applies to:

auto_rebuild_grace_period

ocsp_expiry

delta_rebuild_interval

expiry

Sure! I did update this on a few other PRs. So, I forgot to go back and do it here.

With regard to this, the user sees these as string, e.g. 15m and then it gets parsed into a duration when it's handled. Shouldn't it reflect what the user sees or should it be a duration?
https://developer.hashicorp.com/vault/api-docs/secret/pki#expiry

builtin/logical/pki/path_config_ca.go

Co-authored-by: Anton Averchenkov <[email protected]>

cipherboy · 2022-12-21T13:23:33Z

builtin/logical/pki/path_config_urls.go

+							},
+							"enable_templating": {
+								Type: framework.TypeBool,
+								Description: `Whether or not to enable templating of the


It might be good to reuse descriptions of response/parmas especially for these configs, but we can do the refactoring after this merges.

Yes! That works for me. I tried to keep them consistent. I can definitely update these with better ones after the fact.

builtin/logical/pki/integration_test.go

averche

Thanks for adding the tests! Left a couple comments, otherwise LGTM

averche · 2023-02-06T22:35:41Z

builtin/logical/pki/path_config_crl.go

+						Description: "OK",
+						Fields: map[string]*framework.FieldSchema{
+							"expiry": {
+								Type: framework.TypeString,


Agreed, since we have the tests now, would it be possible to try it out with TypeTime and TypeDurationSecond?

vault/sdk/framework/openapi.go

Lines 805 to 807 in 40f599b

case TypeTime:

ret.baseType = "string"

ret.format = "date-time"

vault/sdk/framework/openapi.go

Lines 785 to 787 in 40f599b

case TypeDurationSecond, TypeSignedDurationSecond:

ret.baseType = "integer"

ret.format = "seconds"

averche · 2023-02-06T22:42:13Z

builtin/logical/pki/path_config_crl.go

+						Description: "OK",
+						Fields: map[string]*framework.FieldSchema{
+							"expiry": {
+								Type: framework.TypeString,


Same comment applies to:

auto_rebuild_grace_period

ocsp_expiry

delta_rebuild_interval

expiry

averche

LGTM 👍

AnPucel added 9 commits December 13, 2022 15:21

First response

7af2481

Adding more resposne structures

6af5640

All config ca response structures

2f337ec

Format

761de13

Add response structures

8b1d9c4

Adding path_config_crl

ded7033

Formatting & Fixing update/create operations

551b128

Removing response structures for update/create

12bb8df

Adding path config urls

9e2e9a0

AnPucel requested a review from a team December 14, 2022 19:58

cipherboy reviewed Dec 14, 2022

View reviewed changes

builtin/logical/pki/path_config_urls.go Outdated Show resolved Hide resolved

cipherboy requested a review from a team December 14, 2022 20:34

AnPucel commented Dec 14, 2022

View reviewed changes

builtin/logical/pki/path_config_ca.go Outdated Show resolved Hide resolved

AnPucel added 5 commits December 14, 2022 15:24

Adding responses to update & create for config_ca

d8ea655

Adding response for udpate

6862724

Adding response structure to update/create

841ce1a

update/create response struct config_urls

9c73e85

Changelog

be28b33

Adding last descriptions

f4b1b83

averche approved these changes Dec 15, 2022

View reviewed changes

builtin/logical/pki/path_config_urls.go Outdated Show resolved Hide resolved

builtin/logical/pki/path_config_crl.go Outdated Show resolved Hide resolved

dhuckins approved these changes Dec 16, 2022

View reviewed changes

Description change

090a660

Co-authored-by: Anton Averchenkov <[email protected]>

cipherboy reviewed Dec 21, 2022

View reviewed changes

cipherboy approved these changes Dec 21, 2022

View reviewed changes

AnPucel added 3 commits January 3, 2023 12:42

Add required values

e9e577e

Add defaults to config_ca

5a1d1f0

Add required fields

706942a

AnPucel added this to the 1.13.0-rc1 milestone Jan 9, 2023

AnPucel force-pushed the anpucel/PKIResponseStructures branch from de44314 to 4548149 Compare January 27, 2023 04:15

vercel bot deployed to Preview January 27, 2023 04:22 View deployment

AnPucel added 8 commits January 31, 2023 13:06

Unstashing tests

7993adb

removing test

f08416e

Formatting

ba80daf

Reverting test change

46c84a9

Wrong file

822a48b

Tests caught this

1a54ac2

remove comment

3a41092

Running go fmt

b431374

averche reviewed Feb 3, 2023

View reviewed changes

builtin/logical/pki/integration_test.go Outdated Show resolved Hide resolved

AnPucel added 7 commits February 3, 2023 13:23

Merge branch 'main' into anpucel/PKIResponseStructures

d5f5983

config/urls

195d7d1

test config/ca

88bb28a

config/cluster test

d70e348

Missed some parameters

911aed4

more tests

fec0a39

Update tests

3972a2e

averche reviewed Feb 6, 2023

View reviewed changes

averche mentioned this pull request Feb 6, 2023

PKI Response Structures Part 2 #18479

Merged

AnPucel added 2 commits February 13, 2023 11:06

Add read check

05f61f9

Read test

39b1e5d

AnPucel modified the milestones: 1.13.0-rc1, 1.14 Feb 13, 2023

averche approved these changes Feb 15, 2023

View reviewed changes

This was referenced Feb 15, 2023

Adding PKI Responses 3 #18596

Merged

PKI Responses Part 4 #18612

Merged

AnPucel merged commit d09e02a into main Feb 15, 2023

AnPucel deleted the anpucel/PKIResponseStructures branch April 4, 2023 19:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Response Structures to PKI Config #18376

Adding Response Structures to PKI Config #18376

AnPucel commented Dec 14, 2022 •

edited

Loading

dhuckins commented Dec 15, 2022

AnPucel commented Dec 15, 2022 •

edited

Loading

averche left a comment

dhuckins Dec 15, 2022

cipherboy Dec 16, 2022 •

edited

Loading

averche Feb 6, 2023

averche Feb 6, 2023

AnPucel Feb 6, 2023

AnPucel Feb 13, 2023

cipherboy Dec 21, 2022

AnPucel Jan 9, 2023

averche left a comment

averche Feb 6, 2023

averche Feb 6, 2023

averche left a comment

	case TypeTime:
	ret.baseType = "string"
	ret.format = "date-time"

	case TypeDurationSecond, TypeSignedDurationSecond:
	ret.baseType = "integer"
	ret.format = "seconds"

Adding Response Structures to PKI Config #18376

Adding Response Structures to PKI Config #18376

Conversation

AnPucel commented Dec 14, 2022 • edited Loading

dhuckins commented Dec 15, 2022

AnPucel commented Dec 15, 2022 • edited Loading

averche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cipherboy Dec 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

averche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

averche left a comment

Choose a reason for hiding this comment

AnPucel commented Dec 14, 2022 •

edited

Loading

AnPucel commented Dec 15, 2022 •

edited

Loading

cipherboy Dec 16, 2022 •

edited

Loading