Fix SSL certificate handling in PortainerClientConfig for production profile #156

Closed
opened 2026-07-09 20:54:00 +00:00 by Polly · 1 comment
Member

Description

When the portainer-automation service runs with the prod profile and connects to the production Portainer at https://192.168.0.100:9443 (self-signed SSL certificate), I want the HTTPS client to trust that certificate, so the SSL handshake succeeds and the service can authenticate and deploy stacks to production.

Root cause: PortainerClientConfig.java has a @Profile("!local") bean that applies strict SSL validation. Since prod is not local, it matches this profile and uses a default RestTemplate which rejects the self-signed cert. The local profile has a trust-all SSL bean, but prod has no equivalent.

Error observed: (bad_certificate) Failed to parse server certificates

Acceptance Criteria

Happy path — Prod profile uses trust-all SSL:
GIVEN the prod Spring profile is active
WHEN the portainerRestTemplate bean is created
THEN it uses a trust-all SSL configuration (same as local) and successfully connects to https://192.168.0.100:9443

Edge case — Non-local, non-prod profiles still validate SSL:
GIVEN a profile like ci or test is active
WHEN the portainerRestTemplate bean is created
THEN it uses strict SSL validation (standard RestTemplate)

Code quality — DRY trust-all logic:
GIVEN the local and prod profiles both need trust-all SSL
WHEN the code is reviewed
THEN the trust-all SSL RestTemplate creation logic is extracted into a shared private/helper method to avoid duplication

Technical Notes

Changes required in PortainerClientConfig.java:

  1. Add a @Profile("prod") bean that creates a trust-all SSL RestTemplate (same logic as the local bean) — annotated with @Bean(name = "portainerRestTemplate")

  2. Update the @Profile("!local") bean annotation to @Profile("!local & !prod") (Spring expression language) so it no longer catches the prod profile

  3. Extract the trust-all SSL RestTemplate creation into a shared private method (e.g., createTrustAllRestTemplate()) used by both the local and prod beans

File to modify:

  • portainer-automation/src/main/java/com/hithomelabs/portainer/config/PortainerClientConfig.java

Testing:

  • Unit test: verify correct bean is returned for each profile (local, prod, ci, test)
  • Manual test: deploy PA with SPRING_PROFILES_ACTIVE=prod and confirm SSL handshake succeeds

Dependencies

  • Blocks: #140 (prod deployment workflow needs working SSL)
  • Related to: #134 (missing profile configs — ensures prod profile is fully wired)

Story Points: 3

Priority: high

## Description When the portainer-automation service runs with the `prod` profile and connects to the production Portainer at `https://192.168.0.100:9443` (self-signed SSL certificate), I want the HTTPS client to trust that certificate, so the SSL handshake succeeds and the service can authenticate and deploy stacks to production. **Root cause**: `PortainerClientConfig.java` has a `@Profile("!local")` bean that applies strict SSL validation. Since `prod` is not `local`, it matches this profile and uses a default `RestTemplate` which rejects the self-signed cert. The `local` profile has a trust-all SSL bean, but `prod` has no equivalent. **Error observed**: `(bad_certificate) Failed to parse server certificates` ## Acceptance Criteria **Happy path — Prod profile uses trust-all SSL:** GIVEN the `prod` Spring profile is active WHEN the `portainerRestTemplate` bean is created THEN it uses a trust-all SSL configuration (same as `local`) and successfully connects to `https://192.168.0.100:9443` **Edge case — Non-local, non-prod profiles still validate SSL:** GIVEN a profile like `ci` or `test` is active WHEN the `portainerRestTemplate` bean is created THEN it uses strict SSL validation (standard `RestTemplate`) **Code quality — DRY trust-all logic:** GIVEN the `local` and `prod` profiles both need trust-all SSL WHEN the code is reviewed THEN the trust-all SSL `RestTemplate` creation logic is extracted into a shared private/helper method to avoid duplication ## Technical Notes ### Changes required in `PortainerClientConfig.java`: 1. **Add** a `@Profile("prod")` bean that creates a trust-all SSL `RestTemplate` (same logic as the `local` bean) — annotated with `@Bean(name = "portainerRestTemplate")` 2. **Update** the `@Profile("!local")` bean annotation to `@Profile("!local & !prod")` (Spring expression language) so it no longer catches the `prod` profile 3. **Extract** the trust-all SSL `RestTemplate` creation into a shared private method (e.g., `createTrustAllRestTemplate()`) used by both the `local` and `prod` beans ### File to modify: - `portainer-automation/src/main/java/com/hithomelabs/portainer/config/PortainerClientConfig.java` ### Testing: - Unit test: verify correct bean is returned for each profile (`local`, `prod`, `ci`, `test`) - Manual test: deploy PA with `SPRING_PROFILES_ACTIVE=prod` and confirm SSL handshake succeeds ## Dependencies - Blocks: #140 (prod deployment workflow needs working SSL) - Related to: #134 (missing profile configs — ensures `prod` profile is fully wired) ## Story Points: 3 ## Priority: high
Polly added this to the Portainer Automation — Production & Parameterization milestone 2026-07-09 20:54:00 +00:00
Polly added the
docker
priority:high
story-points:3
blocks:#140
labels 2026-07-09 20:54:10 +00:00
Linus closed this issue 2026-07-17 13:42:50 +00:00
Member

🏛️ Architectural Review: Developer's Solution (PR #157) vs. Proposed Alternatives

Requested by: @hitanshu — verify the developer's fix against the architect's alternatives.


Verdict Summary

The developer's fix is CORRECT for the immediate blocker, but it implements Option A (the quick profile-based fix) rather than my recommended Option B (property-driven SSL). One latent bug I flagged remains in the merged code.

Aspect Assessment
Resolves the prod blocker (#156)? Yesprod profile now gets trust-all SSL
Matches issue #156's technical notes? Yes (Polly's notes prescribed this exact approach)
DRY code quality Good — buildTrustAllRestTemplate() extraction is clean
Matches architect's recommended approach? No — implemented Option A, not recommended Option B
Latent default-profile bug ⚠️ Still present in merged code

📋 What the Developer Did (PR #157, merged)

@Profile("local")   buildTrustAllRestTemplate()          // trust-all
@Profile("prod")    buildTrustAllRestTemplate()          // trust-all (NEW)
@Profile("!local & !prod")  new RestTemplate()           // strict

This is exactly Option A from my architecture review on #124 (comment #2469): a profile-based fix that adds @Profile("prod") and negates it out of the strict bean.


⚠️ The Latent Bug — Still in Main

In my review (comment #2469 on #124) I flagged that negation-based profiling breaks the default profile:

"the default profile (when no SPRING_PROFILES_ACTIVE is set) loads application.propertieshttps://192.168.0.100:9442 (self-signed) → strict RestTemplate → same bad_certificate failure."

This is unchanged in the merged code:

Active Profile Base URL (self-signed?) SSL Bean Result
local devdocker.hithomelabs.com (self-signed) trust-all
prod 192.168.0.100:9443 (self-signed) trust-all
default (none) 192.168.0.100:9442 (self-signed) strict bad_certificate
ci portainer-test:9000 (HTTP) strict

Risk level: Low today (all real deployments use explicit profiles), but a footgun — anyone running the jar without SPRING_PROFILES_ACTIVE (e.g., local dev, debugging, or a future forgotten deployment) hits the exact error this issue was meant to fix.


🧩 Why the Developer Chose Option A

Timeline (2026-07-09):

  1. 20:54#156 created with technical notes prescribing the profile-based fix
  2. 20:55:38 — My architecture review posted on #124 recommending Option B (property-driven)
  3. 20:57:16 — PR #157 opened implementing Option A (per #156's notes)
  4. 20:58:59 — Merged

Conclusion: The developer followed the issue's technical notes faithfully. The timing (PR opened ~2 min after my review) means the recommendation likely wasn't seen before merge. This is a process gap, not a developer error.


📌 Recommendation (Low-Effort Follow-up)

  1. Accept the merged fix — it unblocks #140 and satisfies #156's acceptance criteria. No revert needed.
  2. Add a small hardening follow-up (Effort: XS–S):
    • Set portainer.trust-all-ssl=true in default application.properties (dev base URL is self-signed), or
    • Add SPRING_PROFILES_ACTIVE=prod explicitly to the prod docker-compose service (belt-and-suspenders)
  3. Reference #133 (SSL profile bean tests) to cover the default-profile case so this regression is caught by tests, not discovered in prod.

🏗️ For Future SSL Work

When the next profile needs SSL handling, migrate to the property-driven approach (portainer.trust-all-ssl flag) I recommended in comment #2469 on #124 — it eliminates the whole class of @Profile negation bugs.


Architect verdict posted by Archie (Architect Agent)
Date: 2026-08-02

## 🏛️ Architectural Review: Developer's Solution (PR #157) vs. Proposed Alternatives **Requested by**: @hitanshu — verify the developer's fix against the architect's alternatives. --- ### ✅ Verdict Summary **The developer's fix is CORRECT for the immediate blocker, but it implements Option A (the quick profile-based fix) rather than my recommended Option B (property-driven SSL). One latent bug I flagged remains in the merged code.** | Aspect | Assessment | |--------|-----------| | Resolves the prod blocker (#156)? | ✅ **Yes** — `prod` profile now gets trust-all SSL | | Matches issue #156's technical notes? | ✅ Yes (Polly's notes prescribed this exact approach) | | DRY code quality | ✅ Good — `buildTrustAllRestTemplate()` extraction is clean | | Matches architect's recommended approach? | ❌ **No** — implemented Option A, not recommended Option B | | Latent default-profile bug | ⚠️ **Still present** in merged code | --- ### 📋 What the Developer Did (PR #157, merged) ```java @Profile("local") → buildTrustAllRestTemplate() // trust-all @Profile("prod") → buildTrustAllRestTemplate() // trust-all (NEW) @Profile("!local & !prod") → new RestTemplate() // strict ``` This is exactly **Option A** from my architecture review on #124 (comment #2469): a profile-based fix that adds `@Profile("prod")` and negates it out of the strict bean. --- ### ⚠️ The Latent Bug — Still in Main In my review (comment #2469 on #124) I flagged that negation-based profiling breaks the **default profile**: > *"the default profile (when no SPRING_PROFILES_ACTIVE is set) loads `application.properties` → `https://192.168.0.100:9442` (self-signed) → strict RestTemplate → same `bad_certificate` failure."* This is **unchanged** in the merged code: | Active Profile | Base URL (self-signed?) | SSL Bean | Result | |----------------|------------------------|----------|--------| | `local` | `devdocker.hithomelabs.com` (self-signed) | trust-all | ✅ | | `prod` | `192.168.0.100:9443` (self-signed) | trust-all | ✅ | | **default (none)** | `192.168.0.100:9442` (self-signed) | **strict** | ❌ `bad_certificate` | | `ci` | `portainer-test:9000` (HTTP) | strict | ✅ | **Risk level**: Low **today** (all real deployments use explicit profiles), but a footgun — anyone running the jar without `SPRING_PROFILES_ACTIVE` (e.g., local dev, debugging, or a future forgotten deployment) hits the exact error this issue was meant to fix. --- ### 🧩 Why the Developer Chose Option A Timeline (2026-07-09): 1. `20:54` — #156 created with technical notes **prescribing the profile-based fix** 2. `20:55:38` — My architecture review posted on #124 recommending **Option B (property-driven)** 3. `20:57:16` — PR #157 opened implementing **Option A** (per #156's notes) 4. `20:58:59` — Merged **Conclusion**: The developer followed the issue's technical notes faithfully. The timing (PR opened ~2 min after my review) means the recommendation likely wasn't seen before merge. This is a process gap, not a developer error. --- ### 📌 Recommendation (Low-Effort Follow-up) 1. **Accept the merged fix** — it unblocks #140 and satisfies #156's acceptance criteria. No revert needed. 2. **Add a small hardening follow-up** (Effort: XS–S): - Set `portainer.trust-all-ssl=true` in default `application.properties` (dev base URL is self-signed), **or** - Add `SPRING_PROFILES_ACTIVE=prod` explicitly to the prod docker-compose service (belt-and-suspenders) 3. **Reference #133** (SSL profile bean tests) to cover the default-profile case so this regression is caught by tests, not discovered in prod. --- ### 🏗️ For Future SSL Work When the next profile needs SSL handling, migrate to the property-driven approach (`portainer.trust-all-ssl` flag) I recommended in comment #2469 on #124 — it eliminates the whole class of `@Profile` negation bugs. --- *Architect verdict posted by Archie (Architect Agent)* *Date: 2026-08-02*
Sign in to join this conversation.
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Hithomelabs/CFTunnels#156
No description provided.