Fix cloud provider discovery logic

[amirmalka]

PR Type:

Refactoring

---

PR Description:

This PR refactors the cloud provider discovery logic in the Kollector application. The cloud provider was previously taken from the config, but this has been removed in recent versions. Instead, the cloud provider is now set once in a global variable upon initialization. The main changes include:

• Addition of a new function to set the cloud provider.
• Modification of the global variables to include the cloud provider.
• Adjustment of the installation data to use the new global cloud provider variable.
• Update of the tests to reflect these changes.
• Addition of error handling for setting the cloud provider in the watch handler.
• Update of the project dependencies.

---

PR Main Files Walkthrough:

files:

• watch/cloudprovider.go: Added a new function 'setCloudProvider' that fetches the list of nodes and sets the global 'cloudProvider' variable.
• watch/globals.go: Added a new global variable 'cloudProvider'.
• watch/jsonformat.go: Replaced 'config.ClusterProvider' with the global variable 'cloudProvider' in the 'setInstallationData' function.
• watch/jsonformat_test.go: Adjusted the tests to use the global 'cloudProvider' variable instead of 'config.ClusterProvider'.
• watch/watchhandler.go: Added error handling for setting the cloud provider during the creation of the watch handler.
• go.mod: Added a new dependency 'github.com/aws/aws-sdk-go'.
• go.sum: Updated the project dependencies.

---

User Description:

Kollector was taking the cloud provider from config (which was set in the Kubescape operator helm chart) but it was removed in recent versions.

Instead, we will set it once in a global var upon initialization.

[codiumai-pr-agent]

PR Analysis

• 🎯 Main theme: Refactoring cloud provider discovery logic
• 📝 PR summary: This PR refactors the cloud provider discovery logic in the Kollector application. The cloud provider was previously taken from the config, but this has been removed in recent versions. Instead, the cloud provider is now set once in a global variable upon initialization. The PR includes the addition of a new function to set the cloud provider, modification of the global variables to include the cloud provider, adjustment of the installation data to use the new global cloud provider variable, and update of the tests to reflect these changes.
• 📌 Type of PR: Refactoring
• 🧪 Relevant tests added: Yes
• ⏱️ Estimated effort to review [1-5]: 3, because the PR includes changes in multiple files and the logic of setting the cloud provider has been significantly changed. However, the changes are straightforward and well explained.
• 🔒 Security concerns: No security concerns found

PR Feedback

• 💡 General suggestions: The PR is well structured and the changes are clearly explained. The use of a global variable for the cloud provider is a good approach as it reduces the need for repeated configuration. However, it would be beneficial to ensure that the global variable is thread-safe, as multiple goroutines may access and change it simultaneously.

• 🤖 Code feedback:

  • relevant file: watch/cloudprovider.go
    suggestion: Consider handling the case where the nodeList is empty or nil. This could happen if there are no nodes in the cluster or if there is an issue with the Kubernetes API. In such cases, the cloudProvider variable would be set to an empty string, which might not be the desired behavior. [important]
    relevant line: cloudProvider = cloudsupport.GetCloudProvider(nodeList)

  • relevant file: watch/watchhandler.go
    suggestion: The error from setCloudProvider is logged but not returned. This might lead to issues if the cloud provider cannot be set but the application continues to run. Consider returning the error to the caller to handle it appropriately. [important]
    relevant line: if err = setCloudProvider(k8sAPiObj); err != nil {

  • relevant file: watch/jsonformat_test.go
    suggestion: The global variable cloudProvider is set directly in the test. Consider using a setup function to initialize this variable to ensure isolation between tests. [medium]
    relevant line: cloudProvider = "test"

  • relevant file: go.mod
    suggestion: The PR introduces a new dependency 'github.com/aws/aws-sdk-go'. Make sure this dependency is necessary and that it doesn't introduce any security vulnerabilities. [medium]
    relevant line: github.com/aws/aws-sdk-go v1.44.312 // indirect

How to use

Instructions

To invoke the PR-Agent, add a comment using one of the following commands:
/review: Request a review of your Pull Request.
/describe: Update the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
/ask <QUESTION>: Ask a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.
/add_docs: Generate docstring for new components introduced in the PR.
/generate_labels: Generate labels for the PR based on the PR's contents.
see the tools guide for more details.

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

[github-actions]

Summary:

• License scan: success
• Credentials scan: success
• Vulnerabilities scan: success
• Unit test: success
• Go linting: success