Introduce support for Catalogs that enable specifying KRMFunctions #5507
Conversation
|
This PR has multiple commits, and the default merge method is: merge. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
the
needs-rebase
This commit introduces a Catalog API, and allows the kustomize command to accept a list of paths that resolve to a catalog file. Signed-off-by: Varsha Prasad Narsing <[email protected]>
This commit modifies the logic to read function specs from catalog in addition to the annotations. This feature is currently enabled only with transformers. Signed-off-by: Varsha Prasad Narsing <[email protected]>
varshaprasad96
force-pushed
the
introduce/catalog-api
branch
from
4ccb1bf to
5a148d7
Compare
k8s-ci-robot
added
size/XXL
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: varshaprasad96 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
Apologies for a bunch of lint fixes in this PR to make the linter happy. The tests which had been failing initially on the PR having been passing locally. The recent push should re-trigger the tests to ensure that it is not a flake. |
|
/cc |
| apiVersion: example.com/v1 | ||
| kind: Example | ||
| metadata: | ||
| name: test | ||
| annotations: | ||
| config.kubernetes.io/function: |- | ||
| container: | ||
| image: foo:v1.0.0 |
There was a problem hiding this comment.
I'm having a bit of difficulty understanding this test case. I read the KEP and the wording in the Determining the Function to Execute section suggests that the current use case with the runtime field should take precedence over the new Catalog option, and still require the --enable-alpha-plugins and --enable-exec flags.
Is config.kubernetes.io/function equivalent to the runtime function mentioned by the KEP in this instance?
| if !assert.NoError(t, err) { | ||
| t.FailNow() | ||
| } | ||
| if !assert.Equal(t, | ||
| strings.TrimSpace(tt.expectedFn), | ||
| strings.TrimSpace(string(b))) { | ||
| t.FailNow() | ||
| } |
There was a problem hiding this comment.
This is more code-style-related, so feel free to disregard if you have a preference: could we use require.NoError() and require.Equal() for brevity in these assertions?
How do you feel about splitting the formatting changes that happened in files unrelated to this implementation into a new PR? |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
needs-rebase
| content, err := ldr.Load(path) | ||
| if err != nil { | ||
| return nil, errors.WrapPrefixf(err, "unable to load catalog path") | ||
| } | ||
|
|
||
| j, err := yaml.YAMLToJSON(content) | ||
| if err != nil { | ||
| return nil, errors.WrapPrefixf(err, "invalid catalog") | ||
| } | ||
|
|
||
| dec := json.NewDecoder(bytes.NewReader(j)) | ||
| dec.DisallowUnknownFields() | ||
|
|
||
| var c framework.Catalog | ||
| if err := dec.Decode(&c); err != nil { | ||
| return nil, errors.WrapPrefixf(err, "unable to find catalog") | ||
| } | ||
| catalogs = append(catalogs, c) |
There was a problem hiding this comment.
Better to move this logic to a separate function for better readability, keeping the methods short and better testing.
| if krmFunc.Group == resGroup && krmFunc.Names.Kind == resKind { | ||
| for _, version := range krmFunc.Versions { | ||
| if version.Name == resVersion { | ||
| return &version.Runtime, nil | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
There is deep nesting here. Better to move this section to a different function with a descriptive method name.
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
k8s-ci-robot
added
the
lifecycle/stale
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
k8s-ci-robot
added
lifecycle/rotten
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close |
|
@k8s-triage-robot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
This PR adds support to Kustomize Function catalogs as mentioned in the KEP: https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/2906-kustomize-function-catalog.
This change introduces an option for users to specify path to their catalog configuration file in
kustomization.yaml. It introduces logic to fetch the function spec from an individual catalog whose spec contains a GVK matches the resource or from the annotation. This feature is currently supported only in Transformers.Follow up: