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

feat: add proxmox integration #1752

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from
Draft

Conversation

manuel-rw
Copy link
Member


Homarr

Thank you for your contribution. Please ensure that your pull request meets the following pull request:

  • Builds without warnings or errors (pnpm buid, autofix with pnpm format:fix)
  • Pull request targets dev branch
  • Commits follow the conventional commits guideline
  • No shorthand variable names are used (eg. x, y, i or any abbrevation)

@manuel-rw manuel-rw linked an issue Dec 22, 2024 that may be closed by this pull request
Copy link

deepsource-io bot commented Dec 22, 2024

Here's the code health analysis summary for commits d34d665..8e67ce9. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript❌ Failure
❗ 4 occurences introduced
View Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.


export class ProxmoxIntegration extends Integration {
public async testConnectionAsync(): Promise<void> {
process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0"; // TODO: Can we improve this?

Check failure

Code scanning / CodeQL

Disabling certificate validation High

Disabling certificate validation is strongly discouraged.

Copilot Autofix AI 14 days ago

To fix the problem, we need to ensure that TLS certificate validation is not disabled. Instead of setting process.env.NODE_TLS_REJECT_UNAUTHORIZED to "0", we should allow the default behavior of Node.js to validate certificates. If there is a need to handle self-signed certificates or certificates from a private CA, we should configure the application to trust those certificates explicitly.

The best way to fix this without changing existing functionality is to remove the line that sets process.env.NODE_TLS_REJECT_UNAUTHORIZED to "0". If necessary, we can add logic to handle custom certificates securely.

Suggested changeset 1
packages/integrations/src/proxmox/proxmox-integration.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/integrations/src/proxmox/proxmox-integration.ts b/packages/integrations/src/proxmox/proxmox-integration.ts
--- a/packages/integrations/src/proxmox/proxmox-integration.ts
+++ b/packages/integrations/src/proxmox/proxmox-integration.ts
@@ -7,3 +7,2 @@
   public async testConnectionAsync(): Promise<void> {
-    process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0"; // TODO: Can we improve this?
     const proxmox = this.getPromoxApi();
EOF
@@ -7,3 +7,2 @@
public async testConnectionAsync(): Promise<void> {
process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0"; // TODO: Can we improve this?
const proxmox = this.getPromoxApi();
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 21.55% 7796 / 36167
🔵 Statements 21.55% 7796 / 36167
🔵 Functions 24.61% 284 / 1154
🔵 Branches 61.79% 859 / 1390
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
apps/nextjs/src/app/[locale]/manage/integrations/_components/secrets/integration-secret-icons.ts 0% 0% 0% 0% 1-12
packages/definitions/src/integration.ts 98.65% 100% 20% 98.65% 157, 160
packages/integrations/src/index.ts 100% 100% 100% 100%
packages/integrations/src/base/creator.ts 57.77% 100% 0% 57.77% 30-39, 42-53
packages/integrations/src/proxmox/proxmox-integration.ts 9.67% 100% 0% 9.67% 8-11, 14-60, 63-68
Generated in workflow #4270 for commit 8e67ce9 by the Vitest Coverage Report Action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: add proxmox integration
2 participants