Skip to content
This repository was archived by the owner on Oct 29, 2025. It is now read-only.

Conversation

@natasha41575
Copy link
Contributor

@natasha41575 natasha41575 commented Mar 4, 2022

Recreating #8 because the rebase made some things weird

This adds a CI check to validate the KRM function metadata based on the openapi schema of KRMFunctionDefinition as defined in the Catalog KEP. I can confirm that it works because it caught some of my mistakes in defining the render-helm-chart function definition

/cc @KnVerey
/cc @jeremyrickard
/cc @mengqiy

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 4, 2022
openAPIV3Schema:
description: openAPIV3Schema is the OpenAPI v3 schema to use for validation
# kube-openapi validation doesn't support references, and inlining this ref is extremely tedious
# $ref: "#/definitions/io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.JSONSchemaProps"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copying from my other comment:

It appears that kube-openapi/validate.AgainstSchema doesn't support references and inlining this reference here will be a lot of tedious work, so I commented it out the reference.

Any errors in this field will be caught by lines 59-61 of tests/validate-metadata_test.go, which just tries to unmarshall the function metadata into the corresponding struct. The errors won't be as user-friendly as the errors caught by kube-openapi, but AFAICT it still catches most of them.

Copy link

Choose a reason for hiding this comment

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

@natasha41575 I'd suggest filing an issue in upstream.

@natasha41575 natasha41575 marked this pull request as draft March 4, 2022 05:01
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 4, 2022
@natasha41575 natasha41575 force-pushed the verifyfunctionmetadata branch from 77c22f4 to a0ecb3e Compare March 4, 2022 05:04
@natasha41575 natasha41575 marked this pull request as ready for review March 4, 2022 05:04
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 4, 2022
Copy link

@mengqiy mengqiy left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 4, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mengqiy, natasha41575

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [mengqiy,natasha41575]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 8ab2497 into kubernetes-retired:main Mar 4, 2022
@natasha41575 natasha41575 deleted the verifyfunctionmetadata branch March 4, 2022 18:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants