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

[docs] Convention Server Open API spec #74

Merged
merged 13 commits into from
Apr 26, 2022

Conversation

katmutua
Copy link
Contributor

@katmutua katmutua commented Apr 7, 2022

Pull request

Include an Open API spec.

What this PR does / why we need it

Create a language agnostic specification on how convention authors can create convention servers.

Which issue(s) this PR fixes

Addresses issue 59

Signed-off-by: Jackline Mutua <jmutua@vmware.com>
@katmutua katmutua changed the title Document the webhook contract Draft: Document the webhook contract Apr 7, 2022
Copy link
Contributor

@scothis scothis left a comment

Choose a reason for hiding this comment

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

Documenting the helpers in the webhook package is useful, the goal for #59 is to document the wire-level mechanical interface.

There are convention authors that don't want to use golang. They should be able to write a convention server in any language as long as they conform to the webhook contract. OpenAPI gives us a language independent way to describe the http request/response that the server needs to implement in order to be compatible.

The ./webhook golang package is an implementation of that contract.

docs/design.md Outdated Show resolved Hide resolved
docs/design.md Outdated Show resolved Hide resolved
docs/design.md Outdated Show resolved Hide resolved
@katmutua katmutua marked this pull request as draft April 12, 2022 13:56
@katmutua katmutua changed the title Draft: Document the webhook contract Document the webhook contract Apr 12, 2022
dependabot bot and others added 3 commits April 12, 2022 17:13
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 2 to 3.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/master/CHANGELOG.md)
- [Commits](codecov/codecov-action@v2...v3)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Prefixing labels is a best practice and `app.kubernetes.io/component` is
a well known label. Other tools can use this label to identify resources
that belong to Cartographer Conventions.

Also dropping the label value namespace, so `conventions.carto.run` ->
`conventions`.

Signed-off-by: Scott Andrews <andrewssc@vmware.com>
Signed-off-by: Jackline Mutua <jmutua@vmware.com>
Copy link
Contributor

@scothis scothis left a comment

Choose a reason for hiding this comment

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

it's good to see this coming together

api/openapi-spec/conventions-server..yaml Outdated Show resolved Hide resolved
api/openapi-spec/conventions-server..yaml Outdated Show resolved Hide resolved
api/openapi-spec/conventions-server..yaml Outdated Show resolved Hide resolved
type: object
properties:
spec:
$ref: "kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/#pod-v1-core"
Copy link
Contributor

Choose a reason for hiding this comment

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

does this type of ref work? I know JSON Ref can use URLs, this doesn't look like a URL and Kubernetes.io would need to publish the schema for each type in open api.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$ref: "kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/#pod-v1-core"
$ref: "kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#pod-v1-core"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intention was to reference the podTemplateSpec definition from a valid kubernetes openapi spec similar to

components:
  schemas:
    User:
      $ref: 'https://api.example.com/v2/openapi.yaml#/components/schemas/User'

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
api/openapi-spec/conventions-server..yaml Outdated Show resolved Hide resolved
api/openapi-spec/conventions-server..yaml Outdated Show resolved Hide resolved
api/openapi-spec/conventions-server..yaml Outdated Show resolved Hide resolved
api/openapi-spec/conventions-server..yaml Outdated Show resolved Hide resolved
docs/design.md Outdated Show resolved Hide resolved
@katmutua katmutua force-pushed the document-webhook-contract branch 4 times, most recently from fefe229 to 2ef19d8 Compare April 13, 2022 18:09
description: specification of the desired behavior of a pod
metadata:
type: object
properties:
Copy link
Contributor

Choose a reason for hiding this comment

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

type: object
properties:
spec:
type: object
Copy link
Contributor

@scothis scothis Apr 13, 2022

Choose a reason for hiding this comment

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

there are a lot of available fields that go quite deep. Similar to the discussion about the ggcr config, we should consider leaving it open ended as to not artificially restrict new fields from being added in later versions of k8s, and not exposed by a convention server. This relates to #60

Signed-off-by: Jackline Mutua <jmutua@vmware.com>
@codecov-commenter
Copy link

codecov-commenter commented Apr 14, 2022

Codecov Report

Merging #74 (ee60400) into main (bb711a0) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #74   +/-   ##
=======================================
  Coverage   69.32%   69.32%           
=======================================
  Files          17       17           
  Lines         929      929           
=======================================
  Hits          644      644           
  Misses        265      265           
  Partials       20       20           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb711a0...ee60400. Read the comment docs.

@katmutua katmutua force-pushed the document-webhook-contract branch 3 times, most recently from 02ee69b to e441912 Compare April 18, 2022 14:36
Signed-off-by: Jackline Mutua <jmutua@vmware.com>
api/openapi-spec/conventions-server.yaml Outdated Show resolved Hide resolved
api/openapi-spec/conventions-server.yaml Outdated Show resolved Hide resolved
api/openapi-spec/conventions-server.yaml Outdated Show resolved Hide resolved
api/openapi-spec/conventions-server.yaml Show resolved Hide resolved
api/openapi-spec/conventions-server.yaml Outdated Show resolved Hide resolved
api/openapi-spec/conventions-server.yaml Outdated Show resolved Hide resolved
api/openapi-spec/conventions-server.yaml Show resolved Hide resolved
api/openapi-spec/conventions-server.yaml Show resolved Hide resolved
api/openapi-spec/conventions-server.yaml Outdated Show resolved Hide resolved
docs/design.md Outdated Show resolved Hide resolved
Signed-off-by: Jackline Mutua <jmutua@vmware.com>
@katmutua katmutua force-pushed the document-webhook-contract branch 10 times, most recently from fc59f1b to 1096ace Compare April 21, 2022 17:28
Signed-off-by: Jackline Mutua <jmutua@vmware.com>
Signed-off-by: Jackline Mutua <jmutua@vmware.com>
@katmutua katmutua force-pushed the document-webhook-contract branch 6 times, most recently from 07af343 to 19979b6 Compare April 25, 2022 20:59
Signed-off-by: Jackline Mutua <jmutua@vmware.com>
@kkavitha kkavitha self-requested a review April 26, 2022 14:10
@kkavitha
Copy link
Contributor

LGTM

@katmutua katmutua merged commit 4432fbd into vmware-tanzu:main Apr 26, 2022
@katmutua katmutua deleted the document-webhook-contract branch May 18, 2023 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants