Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring in the rest of the API refactor proto and service definitions #1548

Merged
merged 3 commits into from
May 15, 2020

Conversation

azdagron
Copy link
Member

This PR has the rest of the protobufs from the server API refactor branch in preparation for starting the work.

I would have merged that branch directly but there were some small updates in master that weren't backported.

Signed-off-by: Andrew Harding <andrew.harding@hpe.com>
message AttestAgentResponse {
message Result {
// The agent X509-SVID.
spire.types.X509SVID identity = 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We name this as svid in other messages. Do you think that identity is a better choice here?
I'm just looking from a consistency point of view.

Comment on lines 165 to 175
// Required. How long until the token expires (in seconds).
int32 ttl = 3;

// An optional token value to use for the token. Must be unique. If unset,
// the server will generate a value.
string token = 1;

// An optional SPIFFE ID to assign to the agent beyond that given by
// join token attestation. If set, this results in an entry being created
// that maps the attestation assigned agent ID to this ID.
spire.types.SPIFFEID agent_id = 2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: follow the same order as the field number

Suggested change
// Required. How long until the token expires (in seconds).
int32 ttl = 3;
// An optional token value to use for the token. Must be unique. If unset,
// the server will generate a value.
string token = 1;
// An optional SPIFFE ID to assign to the agent beyond that given by
// join token attestation. If set, this results in an entry being created
// that maps the attestation assigned agent ID to this ID.
spire.types.SPIFFEID agent_id = 2;
// An optional token value to use for the token. Must be unique. If unset,
// the server will generate a value.
string token = 1;
// An optional SPIFFE ID to assign to the agent beyond that given by
// join token attestation. If set, this results in an entry being created
// that maps the attestation assigned agent ID to this ID.
spire.types.SPIFFEID agent_id = 2;
// Required. How long until the token expires (in seconds).
int32 ttl = 3;

- rename `identity` field to `svid` in the AttestAgent response
- remove language around "canonical identity"
- fix field ordering on CreateJoinTokenRequest

Signed-off-by: Andrew Harding <andrew.harding@hpe.com>
Signed-off-by: Andrew Harding <andrew.harding@hpe.com>
Copy link
Member

@amartinezfayo amartinezfayo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@azdagron azdagron merged commit 68a7438 into spiffe:master May 15, 2020
@azdagron azdagron deleted the import-api-refactor-protos branch May 15, 2020 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants