Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

api: add schema and stubs for bootstrap token CRUD #384

Merged
merged 11 commits into from
Sep 29, 2022

Conversation

mikemorris
Copy link
Contributor

@mikemorris mikemorris commented Sep 20, 2022

Changes proposed in this PR:

  • Add initial schema and stubs for bootstrap token CRUD operations.
  • Add a new make gen command to install oapi-codegen if necessary and regenerate generated code and golden test files.

How I've tested this PR:

Running make gen and go test ./... in internal/api/v1 to make sure I didn't accidentally break anything.

How I expect reviewers to test this PR:

Is there a less verbose way to nest sub-paths like the token CRUD operations under /gateway/{name}/ in the OpenAPI schema?

Checklist:

  • Tests added
  • CHANGELOG entry added

    Run make changelog-entry for guidance in authoring a changelog entry, and
    commit the resulting file, which should have a name matching your PR number.
    Entries should use imperative present tense (e.g. Add support for...)

@mikemorris mikemorris requested a review from a team September 20, 2022 17:48
go generate ./...
make ctrl-generate
make ctrl-manifests
make gen
Copy link
Member

Choose a reason for hiding this comment

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

👏

application/json:
schema:
"$ref": "#/components/schemas/Error"
"/namespaces/{namespace}/gateways/{name}/tokens/{id}":
Copy link
Member

Choose a reason for hiding this comment

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

Should we have a retrieval endpoint or should we assume that the token is only available on generation? I'm not sure how secure we need these to be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, I wasn't sure about this either, thoughts @andrewstucki?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will keep this for now and remove later if not needed (assuming all of this may be subject to change depending on how our VM/API work evolves), agreed it might be useful for returning metadata about a token like TTL, revocation status, etc.

Copy link
Member

@sarahalsmiller sarahalsmiller left a comment

Choose a reason for hiding this comment

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

Stubs look good to me, just the one question about if we should have the GET endpoint for tokens. We might want to keep it and have it just return some metadata about the tokens, not sure.

@mikemorris mikemorris merged commit b89e19b into main Sep 29, 2022
@mikemorris mikemorris deleted the api/bootstrap-token-crud branch September 29, 2022 17:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants