Refactor CA cert pool handling

[selzoc]

No description provided.

[coderabbitai]

Caution

Review failed

Failed to post review comments

Walkthrough

This pull request enhances TLS certificate handling across the deployment system. Changes include: replacing insecure TLS client creation with CA-based certificate pool initialization in deployment deletion and preparation flows; adding a new SecureTLSClientMatcher() test helper to enforce TLS verification in mock expectations; introducing a CACertPool() method to the Certificate type for PEM-based certificate pool construction; and updating test suites to use the new matcher for stricter TLS validation.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	No description was provided by the author, making it impossible to assess whether it relates to the changeset.	Add a description explaining the purpose of this refactoring, such as why CA cert pool handling was changed and what benefits it provides.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Refactor CA cert pool handling' directly and clearly summarizes the main change: refactoring how CA certificate pools are handled throughout the codebase.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor-cert-pool

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR refactors installation-manifest CA handling by centralizing CA parsing into a reusable helper and switching blobstore HTTP clients from InsecureSkipVerify to verified TLS using either system roots or a manifest-provided CA bundle.

Changes:

• Add Certificate.CACertPool() to parse the installation manifest’s PEM CA into an *x509.CertPool (or return nil to use system roots).
• Update deployment prepare/delete flows to create blobstore clients with CreateDefaultClient(certPool) instead of CreateDefaultClientInsecureSkipVerify().
• Add/adjust tests: new unit tests for CACertPool, plus a gomock matcher and test updates to assert blobstore clients are configured with TLS verification enabled.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
installation/manifest/manifest.go	Introduces CACertPool() helper to parse PEM CA into an *x509.CertPool.
installation/manifest/certificate_test.go	Adds unit coverage for CACertPool() (empty CA, valid PEM, invalid PEM).
cmd/suite_test.go	Adds a gomock matcher used by cmd tests to assert blobstore clients are configured for secure TLS verification.
cmd/deployment_preparer.go	Switches blobstore client creation to verified TLS, using CA cert pool from the installation manifest.
cmd/deployment_deleter.go	Switches blobstore client creation to verified TLS, using CA cert pool derived from the provided CA string.
cmd/deployment_deleter_test.go	Updates blobstore factory expectations to require a secure TLS client.
cmd/create_env_test.go	Updates blobstore factory expectations to require a secure TLS client and sets installationManifest.Cert.CA.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.