kcl-lang / krm-kcl

Kubernetes Resource Model KCL Specification and Integrations including Kubectl, Kustomize, Helm, Helmfile, Crossplane, KPT, etc.
Apache License 2.0
20 stars 13 forks source link

feat: add resource rules to krm-kcl spec #54

Closed shruti2522 closed 6 months ago

shruti2522 commented 6 months ago

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

fix #13

2. What is the scope of this PR (e.g. component or file name):

kcl-lang/krm-kcl/pkg/options/testdata/resource_list

3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):

Added resource matching rules to the krm-kcl spec

4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):

5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:

shruti2522 commented 6 months ago

Please take a look, @Peefy

Peefy commented 6 months ago

I think we should first implement this feature in the corresponding code before modifying the test cases.

shruti2522 commented 6 months ago

I have added the MatchConstraints struct and the function to verify resource matching rules to config.go @Peefy

Peefy commented 6 months ago

Could you please fix the CI?

shruti2522 commented 6 months ago

I have modified the test cases in config_test.go to address the errors, and make test works fine now.

PASS
ok      kcl-lang.io/krm-kcl     7.814s
shruti@fedora:~/cncf/krm-kcl$
shruti2522 commented 6 months ago

I think we should use the resourceRules field directly, and remove matchConstraints to resolve the following errors while testing.

FAIL: TestRunLocalPath (0.00s)
    --- FAIL: TestRunLocalPath/resource_list (0.00s)
        run_test.go:72: TestRunLocalPath() error = yaml: unmarshal errors:
              line 10: cannot unmarshal !!map into []config.ResourceRule, wantErr false

Is this the correct approach @Peefy.

Peefy commented 6 months ago

I suggest keeping this field as it is consistent with Kubernetes CEL, please refer to https://kubernetes.io/docs/reference/access-authn-authz/validating-admission-policy/#creating -A-validatingadmissionpolicy

shruti2522 commented 6 months ago

Any ideas on what might be causing this error @Peefy, since I have tried out correcting any possible syntax or test case error.

shruti2522 commented 6 months ago

Done with the changes @Peefy