Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementing Service Discovery Backend #147

Merged
merged 11 commits into from
Aug 30, 2023

Conversation

amirmalka
Copy link
Contributor

@amirmalka amirmalka commented Aug 22, 2023

PR Type:

Refactoring


PR Description:

This PR refactors the service discovery backend. It updates the import paths to reflect the new location of the backend package and modifies the function signatures to align with the updated backend API. The PR also includes changes to the test files to accommodate the modifications. The main changes occur in the 'backend.go' and 'backend_utils.go' files in the 'adapters/v1' directory.


PR Main Files Walkthrough:

adapters/v1/backend.go: The 'ArmoAdapter' struct has been renamed to 'BackendAdapter'. The import paths have been updated to reflect the new location of the backend package. The function signatures have been modified to align with the updated backend API. The 'NewArmoAdapter' function has been renamed to 'NewBackendAdapter'. The 'getCVEExceptionsFunc', 'httpPostFunc', and 'sendStatusFunc' functions have been updated to use the new backend client.
adapters/v1/backend_utils.go: The import paths have been updated to reflect the new location of the backend package. The function signatures have been modified to align with the updated backend API. The 'sendSummaryAndVulnerabilities', 'postResultsAsGoroutine', 'postResults', 'sendVulnerabilitiesRoutine', and 'sendVulnerabilities' functions have been updated to use the new backend client.
adapters/v1/backend_test.go: The 'ArmoAdapter' struct has been renamed to 'BackendAdapter' in the test file. The 'NewArmoAdapter' function has been renamed to 'NewBackendAdapter'. The 'getCVEExceptionsFunc' function has been updated to use the new backend client.
adapters/v1/backend_utils_test.go: The import paths have been updated to reflect the new location of the backend package.
go.mod: The 'go.mod' file has been updated to include the new backend package.
go.sum: The 'go.sum' file has been updated to include the new backend package.

Signed-off-by: Amir Malka <amirm@armosec.io>
Signed-off-by: Amir Malka <amirm@armosec.io>
@codiumai-pr-agent
Copy link

PR Analysis

  • 🎯 Main theme: Refactoring the backend service discovery
  • 📌 Type of PR: Refactoring
  • Focused PR: True
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • General suggestions: The PR seems to be well-structured and focused on refactoring the backend service discovery. However, it's important to add relevant tests for the new changes to ensure they work as expected and do not break any existing functionality.

  • 🤖 Code feedback:

    • relevant file: adapters/v1/backend_utils.go
      suggestion: It seems like you are using the default HTTP client for making requests. It's generally a good practice to use a custom HTTP client with timeout to prevent potential hang of your application when the server is not responding. [important]
      relevant line: resp, err := a.httpPostFunc(http.DefaultClient, urlBase.String(), map[string]string{"Content-Type": "application/json"}, payload)

    • relevant file: config/config.go
      suggestion: It's a good practice to validate the configuration after loading it. This can help catch any misconfigurations or missing required fields early. [medium]
      relevant line: return config, err

    • relevant file: adapters/v1/backend.go
      suggestion: It's recommended to handle context cancellation properly in your goroutines. You can select on 'ctx.Done()' channel and exit the goroutine when the context is cancelled. [important]
      relevant line: go func(report *v1.ScanResultReport, eventReceiverURL, imagetag string, wlid string, errorChan chan<- error, wg *sync.WaitGroup) {

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: Suggest improvements to the code in the PR.
/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.

Signed-off-by: Amir Malka <amirm@armosec.io>
Signed-off-by: Amir Malka <amirm@armosec.io>
Signed-off-by: Amir Malka <amirm@armosec.io>
Signed-off-by: Amir Malka <amirm@armosec.io>
Signed-off-by: Amir Malka <amirm@armosec.io>
Signed-off-by: Amir Malka <amirm@armosec.io>
Signed-off-by: Amir Malka <amirm@armosec.io>
Signed-off-by: Amir Malka <amirm@armosec.io>
@amirmalka
Copy link
Contributor Author

/describe

@codiumai-pr-agent codiumai-pr-agent bot changed the title Service discovery Implementing Service Discovery Backend Aug 27, 2023
Signed-off-by: Amir Malka <amirm@armosec.io>
@amirmalka amirmalka added the release Create release label Aug 29, 2023
@amirmalka amirmalka merged commit 742b806 into kubescape:main Aug 30, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants