hashgraph / hashgraph/solo-weaver

feat(cli): guard `eso operator install` against an existing cluster-wide ESO

Closed
#887 0 comments 0 reactions 1 assignee Claimed by @Dosik13 View on GitHub
Dominant language
Go
Stars
3
Forks
0
Avg merge
3d 5h
Merged PRs (30d)
47

Description

Part of #377.

## Problem

ESO's CRDs (`ExternalSecret`, `SecretStore`, the generators, …) are **cluster-scoped**, so ESO is
effectively a per-cluster singleton — there can only be one. `eso operator install` does not check
for an ESO that already exists in another namespace, so running it against a second namespace
re-creates those CRDs and fails on Helm ownership metadata, leaving a half-created namespace
behind:

```
✗ Installing External Secrets Operator
common.illegal_argument: ... CustomResourceDefinition
"acraccesstokens.generators.external-secrets.io" ... cannot be imported into the current
release: invalid ownership metadata; annotation validation error:
key "meta.helm.sh/release-namespace" must equal "my-eso": current value is "external-secrets"
```

This is the same class of cryptic, side-effect-leaving failure #883 targets for the
cluster-unreachable case — a different trigger (ESO already present elsewhere), same "fail before
any Helm call with an actionable message" fix. It was flagged in the #670 review and called out as
deferred in PR #828's description.

## Proposed fix

Following #883's preflight approach, and landing in the **same preflight gate** #883 adds (so ESO
install has a single fail-fast pre-check block, not two):

* Before any Helm call, detect an existing ESO release in **any** namespace via `hm.ListAll()`
(returns releases in all states), matching the ESO chart name.
* If ESO already exists in a **different** namespace, **skip/warn explicitly** — do not attempt the
install and do not create the target namespace — with an actionable message naming the existing
namespace and pointing at the existing installation (or `eso operator uninstall`, #671),
attaching a `models.ErrPropertyResolution` hint per repo convention.
* Also surface a stalled/failed ESO release in the **target** namespace (which the deployed-only
`IsInstalled` check does not count as installed) with a "clean it up first" message rather than a
raw release-name collision.

A working implementation + unit tests for this guard already exist from the initial #670
development (before PR #828 was trimmed to a lean scope), so this is largely a re-apply — relocated
into #883's preflight gate.

**Behavior (error, not silent skip):** a namespace mismatch is a *conflict* with what the operator
asked for, so it errors — consistent with the repo's convention for conflicting-state
preconditions (`block node upgrade` when not installed → "cannot upgrade, use install"; daemon
install when already running → "stop it before installing"). A no-op/skip is reserved for the
*already-in-desired-state* case, i.e. installing into the namespace where ESO already runs, which
stays the existing idempotent no-op. This satisfies the "explicitly instead of colliding" ask — an
actionable error is the explicit handling.

**Non-goal:** re-introducing per-namespace ESO installs. ESO is a per-cluster singleton by design;
this issue makes that explicit, it does not try to support two coexisting ESOs.

## Acceptance

- [ ] `eso operator install --namespace ` while ESO already exists elsewhere **fails**
(non-zero) **before** any Helm call, with an actionable message naming the existing
namespace — not the raw CRD-ownership error.
- [ ] The target namespace is **not** created when the guard trips.
- [ ] A stalled/failed ESO release in the target namespace is surfaced with a "clean it up first"
message rather than a raw release-name collision.
- [ ] Installing into the namespace where ESO already runs remains the existing idempotent no-op
(unchanged).
- [ ] The check lives in the shared install preflight gate introduced by #883 (not duplicated in
`installESOChart`).
- [ ] Unit tests cover the different-namespace and stalled-same-namespace cases.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.