feat: WIP add control etc crds #46

XDRAGON2002 · 2023-10-11T03:15:28Z

PR Type:

Enhancement

PR Description:

This PR introduces several new Custom Resource Definitions (CRDs) and their related functionalities:

Control: Represents a control entity in the software composition schema.
Rule: Represents a rule entity in the software composition schema.
Framework: Represents a framework entity in the software composition schema.
Exception: Represents an exception entity in the software composition schema.

For each of these CRDs, the PR includes the necessary conversion functions for the software composition scheme. It also updates the clientset, listers, and informers to handle these new CRDs.

PR Main Files Walkthrough:

files:

pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go: Added conversion functions for the new CRDs (Control, Rule, Framework, Exception).
pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/controlconfiguration.go: Updated the clientset to handle the new ControlConfiguration CRD.
pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/framework.go: Updated the clientset to handle the new Framework CRD.
pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/exception.go: Updated the clientset to handle the new Exception CRD.
pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/rule.go: Updated the clientset to handle the new Rule CRD.
pkg/apis/softwarecomposition/v1beta1/types.go: Added definitions for the new CRDs (Control, Rule, Framework, Exception).
pkg/generated/informers/externalversions/softwarecomposition/v1beta1/controlconfiguration.go: Updated the informers to handle the new ControlConfiguration CRD.
pkg/generated/informers/externalversions/softwarecomposition/v1beta1/exception.go: Updated the informers to handle the new Exception CRD.
pkg/generated/informers/externalversions/softwarecomposition/v1beta1/framework.go: Updated the informers to handle the new Framework CRD.
pkg/generated/informers/externalversions/softwarecomposition/v1beta1/rule.go: Updated the informers to handle the new Rule CRD.

codiumai-pr-agent-free · 2023-10-11T03:17:12Z

PR Analysis

🎯 Main theme: Addition of new Custom Resource Definitions (CRDs) to the software composition
📝 PR summary: This PR introduces new control Custom Resource Definitions (CRDs) in the software composition. The new CRDs include Framework, Control, Rule, Exception, and ControlConfiguration. Each of these CRDs has been defined with respective list types and necessary fields. The changes have been made in the register.go and types.go files under the pkg/apis/softwarecomposition and pkg/apis/softwarecomposition/v1beta1 directories. The PR also updates the k8s.io/apimachinery version in go.mod.
📌 Type of PR: Enhancement
🧪 Relevant tests added: No
⏱️ Estimated effort to review [1-5]: 4, due to the complexity of the changes and the need to understand the context of the new CRDs being introduced.
🔒 Security concerns: No security concerns found

PR Feedback

💡 General suggestions: The PR introduces new CRDs which are a significant change to the system. It would be beneficial to include tests that verify the correct behavior of these new CRDs. Additionally, it would be helpful to provide more context in the PR description about the purpose and usage of these new CRDs.
🤖 Code feedback:
- relevant file: pkg/apis/softwarecomposition/types.go
  suggestion: Consider adding error handling for the case when the object is not of type Framework in the GetAttrs function. This will prevent potential runtime errors. [important]
  relevant line: "+ apiserver, ok := obj.(*softwarecomposition.Framework)"
- relevant file: pkg/apis/softwarecomposition/types.go
  suggestion: It would be beneficial to add validation for the new CRDs in the Validate function. This will ensure that only valid objects are processed further. [important]
  relevant line: "+ return validation.AlwaysValid(framework)"
- relevant file: pkg/apis/softwarecomposition/types.go
  suggestion: Consider adding a comment explaining the purpose and usage of the new CRDs. This will improve code readability and maintainability. [medium]
  relevant line: "+type Framework struct {"
- relevant file: go.mod
  suggestion: Ensure that the update to k8s.io/apimachinery from v0.26.2 to v0.28.2 does not introduce any breaking changes or compatibility issues. [important]
  relevant line: "+ k8s.io/apimachinery v0.28.2"

How to use

To invoke the PR-Agent, add a comment using one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest improvements to the code in the PR. Extended mode employs several calls, and provides a more thorough feedback.
/ask <QUESTION>: Pose a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.

To edit any configuration parameter from configuration.toml, add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, use the /config command.

XDRAGON2002 · 2023-10-11T03:19:09Z

@vladklokun as discussed, I've been working on this for the past week, right now I'm stuck a bit as I just can't seem to be able to successfully run the codegen script hack/update-codegen.sh.

The script always seems to error out/give out warnings? Not even sure what they are, every time I run it but can't seem to generate the required deepcopy methods, also why they are not present in the PR.

I've looked for solutions online but can't seem to get it working still, any help regarding this would be extremely helpful! Thanks!

vladklokun · 2023-10-23T06:47:26Z

The script always seems to error out/give out warnings? Not even sure what they are, every time I run it but can't seem to generate the required deepcopy methods, also why they are not present in the PR.

Yes, the script always generates warnings when running, even after successfully generating deep copy methods, converters and OpenAPI definitions. That’s normal.

You can tell your script ran okay-ish when it generates all the methods you need.

Regarding the missing methods, I know it might sound exotic, but where have you cloned your project? There is a catch with hack/update_codegen.sh that it outputs its definitions in the $HOME/github.com/kubescape/storage/pkg directory, provided you installed the dependencies with the go mod vendor method. To you have generated files in this directory, by chance?

XDRAGON2002 · 2023-10-23T17:37:53Z

I did check this earlier as well, but that doesn't seem to be the case either, any things I should try and troubleshoot?

I was going to take a fresh look at it later again today :)

XDRAGON2002 · 2023-11-09T13:43:21Z

@vladklokun finally got the codegen working, could you maybe take a look and let me know if I've missed something before I go ahead and resolve the merge conflicts?

Thanks!

codiumai-pr-agent-free · 2023-11-09T13:46:35Z

PR Analysis

🎯 Main theme: Adding new Custom Resource Definitions (CRDs) and their related functionalities to the software composition schema.
📝 PR summary: This PR introduces several new Custom Resource Definitions (CRDs) and their related functionalities: Control, Rule, Framework, and Exception. It includes the necessary conversion functions for the software composition scheme and updates the clientset, listers, and informers to handle these new CRDs.
📌 Type of PR: Enhancement
🧪 Relevant tests added: No

How to use

To invoke the PR-Agent, add a comment using one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest improvements to the code in the PR. Extended mode employs several calls, and provides a more thorough feedback.
/ask <QUESTION>: Pose a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.

To edit any configuration parameter from configuration.toml, add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, use the /config command.

vladklokun · 2023-11-28T15:53:40Z

Hi! I finally had a chance to look at the PR. Thank you for your patience.

Type definitions and wiring look good to me.

However, we just merged a PR that includes changes to codegen. Please rebase and try to fix conflicts with the base branch and try to verify that the Custom Resources work. A good start to manually test this would be to create example Custom Resources in the artifacts/{rule,framework,control,...} directory, deploy your APIServer in a cluster using the manifests in the artifacts/example directory and then to create the example Custom Resources you put in the artifacts subdirectories.

XDRAGON2002 · 2023-11-29T22:02:03Z

Sure!

Will do that and get back to you on this.

Thanks!

Signed-off-by: DRAGON <[email protected]>

XDRAGON2002 · 2023-12-11T07:27:28Z

@vladklokun have made the required changes! Thanks!

dwertent · 2023-12-11T08:44:55Z

@XDRAGON2002 Can you please commit the go.sum file as well so the test will pass.

XDRAGON2002 · 2023-12-11T08:59:28Z

@dwertent Completely forgot about that! Thanks! Done!

XDRAGON2002 · 2023-12-11T12:17:21Z

This is weird, the tests still failed due to the go.mod/sum dependency issue, I already did push the updated ones though.

Edit: Most probably got it, I was using go1.20 whereas some new packages require 1.21, did the change updated everything and pushed, should pass now.

vladklokun

Other than formatting and the apimachinery bump, LGTM. But before anyone approves, we need to make sure the apimachinery bump actually works with all of our clients importing the Storage and doesn’t break anything.

go.mod

vladklokun · 2023-12-11T12:37:00Z

pkg/apis/softwarecomposition/types.go

+	metav1.TypeMeta
+	metav1.ListMeta
+	Items []ControlConfiguration
+}


chore: fix line endings in PR

Please see the sign after this line. It indicates that the files have improper line endings at the end: they don’t have the terminating \n characters. I believe it’s because you’re using Visual Studio Code or some other IDE that terminates the files with Windows line endings.

Please refer to this issue: microsoft/vscode#141169

I'll fix this,

also the fact that VSC does this is just concerning, I'd at least hope to have my tooling follow standards :(

Yeah, that’s a common problem. Nonetheless, we could always add a Git pre-commit hook to fix the line endings automatically or a linter to complain about it. But as you can imagine, this is very low on our priorities list :)

Signed-off-by: DRAGON <[email protected]>

XDRAGON2002 · 2023-12-23T09:27:00Z

@vladklokun @dwertent I'm pretty sure the tests will fail again, I can't seem to figure out why it is so.

Any help would be much appreciated!

XDRAGON2002 marked this pull request as draft October 11, 2023 03:15

codiumai-pr-agent-free bot added the enhancement New feature or request label Oct 11, 2023

XDRAGON2002 marked this pull request as ready for review November 9, 2023 13:44

feat: add framework crds

fb0def7

Signed-off-by: DRAGON <[email protected]>

XDRAGON2002 force-pushed the main branch from 6f137f8 to 892fd4b Compare December 11, 2023 06:23

XDRAGON2002 force-pushed the main branch from 892fd4b to e532dce Compare December 11, 2023 08:58

XDRAGON2002 force-pushed the main branch from e532dce to c147118 Compare December 11, 2023 12:27

vladklokun reviewed Dec 11, 2023

View reviewed changes

feat: add generated code

42d37e3

Signed-off-by: DRAGON <[email protected]>

XDRAGON2002 force-pushed the main branch from c147118 to 42d37e3 Compare December 23, 2023 09:24

Merge branch 'main' into main

8a3840b

XDRAGON2002 mentioned this pull request Dec 23, 2023

feat: WIP add CRD support kubescape/kubescape#1565

Closed

XDRAGON2002 changed the title ~~feat: add control etc crds~~ feat: WIP add control etc crds Dec 23, 2023

matthyx closed this Sep 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: WIP add control etc crds #46

feat: WIP add control etc crds #46

XDRAGON2002 commented Oct 11, 2023 •

edited by codiumai-pr-agent-free bot

Loading

codiumai-pr-agent-free bot commented Oct 11, 2023

XDRAGON2002 commented Oct 11, 2023

vladklokun commented Oct 23, 2023

XDRAGON2002 commented Oct 23, 2023

XDRAGON2002 commented Nov 9, 2023

codiumai-pr-agent-free bot commented Nov 9, 2023

vladklokun commented Nov 28, 2023

XDRAGON2002 commented Nov 29, 2023

XDRAGON2002 commented Dec 11, 2023

dwertent commented Dec 11, 2023

XDRAGON2002 commented Dec 11, 2023

XDRAGON2002 commented Dec 11, 2023 •

edited

Loading

vladklokun left a comment

vladklokun Dec 11, 2023 •

edited

Loading

XDRAGON2002 Dec 11, 2023

vladklokun Dec 11, 2023

XDRAGON2002 commented Dec 23, 2023

feat: WIP add control etc crds #46

feat: WIP add control etc crds #46

Conversation

XDRAGON2002 commented Oct 11, 2023 • edited by codiumai-pr-agent-free bot Loading

PR Type:

PR Description:

PR Main Files Walkthrough:

codiumai-pr-agent-free bot commented Oct 11, 2023

PR Analysis

PR Feedback

How to use

XDRAGON2002 commented Oct 11, 2023

vladklokun commented Oct 23, 2023

XDRAGON2002 commented Oct 23, 2023

XDRAGON2002 commented Nov 9, 2023

codiumai-pr-agent-free bot commented Nov 9, 2023

PR Analysis

How to use

vladklokun commented Nov 28, 2023

XDRAGON2002 commented Nov 29, 2023

XDRAGON2002 commented Dec 11, 2023

dwertent commented Dec 11, 2023

XDRAGON2002 commented Dec 11, 2023

XDRAGON2002 commented Dec 11, 2023 • edited Loading

vladklokun left a comment

Choose a reason for hiding this comment

vladklokun Dec 11, 2023 • edited Loading

Choose a reason for hiding this comment

XDRAGON2002 Dec 11, 2023

Choose a reason for hiding this comment

vladklokun Dec 11, 2023

Choose a reason for hiding this comment

XDRAGON2002 commented Dec 23, 2023

XDRAGON2002 commented Oct 11, 2023 •

edited by codiumai-pr-agent-free bot

Loading

XDRAGON2002 commented Dec 11, 2023 •

edited

Loading

vladklokun Dec 11, 2023 •

edited

Loading