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.

Sign up for GitHub

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

Jump to bottom

[docs] Convention Server Open API spec #74

Merged

katmutua merged 13 commits into vmware-tanzu:main from katmutua:document-webhook-contract

Apr 26, 2022

Contributor

katmutua commented Apr 7, 2022 •

edited

Loading

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


          Document the webhook contract

Signed-off-by: Jackline Mutua <[email protected]>

vmwclabot added the cla-not-required label

katmutua changed the title ~~Document the webhook contract~~ Draft: Document the webhook contract

scothis reviewed

View reviewed changes

Contributor

scothis left a comment

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 marked this pull request as draft

April 12, 2022 13:56

katmutua changed the title ~~Draft: Document the webhook contract~~ Document the webhook contract

dependabot bot and others added 3 commits

April 12, 2022 17:13


          Bump codecov/codecov-action from 2 to 3 (vmware-tanzu#72)

ec8da7c

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] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>


          Switch component label to app.kubernetes.io/component (vmware-tanzu#73)

381a307

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 <[email protected]>


          Add an initial draft for the spec

58d1dbb

Signed-off-by: Jackline Mutua <[email protected]>

katmutua force-pushed the document-webhook-contract branch from 35c830a to 58d1dbb Compare

April 12, 2022 22:32

scothis reviewed

View reviewed changes

Contributor

scothis left a comment

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

api/openapi-spec/conventions-server..yaml Outdated

+                    type: object
+                    properties:
+                      spec:
+                        $ref: "kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/#pod-v1-core"

Contributor

scothis Apr 13, 2022

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.

Contributor

scothis Apr 13, 2022

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"

Contributor Author

katmutua Apr 20, 2022

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 force-pushed the document-webhook-contract branch 4 times, most recently from fefe229 to 2ef19d8 Compare

April 13, 2022 18:09

scothis reviewed

View reviewed changes

api/openapi-spec/conventions-server..yaml Outdated Show resolved Hide resolved

scothis reviewed

View reviewed changes

api/openapi-spec/conventions-server..yaml Outdated Show resolved Hide resolved

scothis reviewed

View reviewed changes

api/openapi-spec/conventions-server..yaml Outdated

+                        description: specification of the desired behavior of a pod
+                      metadata:
+                        type: object
+                        properties:

Contributor

scothis Apr 13, 2022

there are many more possible fields https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#ObjectMeta

scothis reviewed

View reviewed changes

api/openapi-spec/conventions-server..yaml Outdated

+                    type: object
+                    properties:
+                      spec:
+                        type: object

Contributor

scothis Apr 13, 2022 •

edited

Loading

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

rszumlakowski reviewed

View reviewed changes

api/openapi-spec/conventions-server..yaml Outdated Show resolved Hide resolved


          Remove kubernetes io reference

453616b

Signed-off-by: Jackline Mutua <[email protected]>

katmutua force-pushed the document-webhook-contract branch from 2ef19d8 to 453616b Compare

April 14, 2022 16:42


          Merge branch 'main' into document-webhook-contract

1815ab6

codecov-commenter commented Apr 14, 2022 •

edited

Loading

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.

rszumlakowski reviewed

View reviewed changes

api/openapi-spec/conventions-server.yaml Outdated Show resolved Hide resolved

katmutua force-pushed the document-webhook-contract branch 3 times, most recently from 02ee69b to e441912 Compare

April 18, 2022 14:36


          Update webhook helper section

026b44f

Signed-off-by: Jackline Mutua <[email protected]>

katmutua force-pushed the document-webhook-contract branch from e441912 to 026b44f Compare

April 18, 2022 14:43

katmutua force-pushed the document-webhook-contract branch from e20899e to 79c721f Compare

April 20, 2022 19:04

scothis reviewed

View reviewed changes

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

katmutua force-pushed the document-webhook-contract branch from 52e711d to 1ea4260 Compare

April 20, 2022 20:25


          Resolve comments and include suggestions

a81c519

Signed-off-by: Jackline Mutua <[email protected]>

katmutua force-pushed the document-webhook-contract branch 10 times, most recently from fc59f1b to 1096ace Compare

April 21, 2022 17:28


          Add detail on how to configure the webhook path

d06b20d

Signed-off-by: Jackline Mutua <[email protected]>

katmutua force-pushed the document-webhook-contract branch from 1096ace to d06b20d Compare

April 21, 2022 18:04


          Add detail on how to configure the webhook path

b444ae0

Signed-off-by: Jackline Mutua <[email protected]>

katmutua force-pushed the document-webhook-contract branch 6 times, most recently from 07af343 to 19979b6 Compare

April 25, 2022 20:59


          Reference the ClusterPodConvetion and associated components

c3af6d0

Signed-off-by: Jackline Mutua <[email protected]>

katmutua force-pushed the document-webhook-contract branch from 19979b6 to c3af6d0 Compare

April 25, 2022 23:22

kkavitha self-requested a review

April 26, 2022 14:10

Contributor

kkavitha commented Apr 26, 2022

LGTM

kkavitha approved these changes

View reviewed changes

katmutua merged commit 4432fbd into vmware-tanzu:main

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

Labels

cla-not-required