Skip to content

chore: deprecate olm proposal#4175

Merged
Skarlso merged 2 commits intomainfrom
gc/chore/deprecate-olm
Dec 4, 2024
Merged

chore: deprecate olm proposal#4175
Skarlso merged 2 commits intomainfrom
gc/chore/deprecate-olm

Conversation

@gusfcarvalho
Copy link
Copy Markdown
Member

This Proposal deprecates OLM builds as a part of our build cycle.

Signed-off-by: Gustavo Carvalho <gusfcarvalho@gmail.com>
@gusfcarvalho gusfcarvalho requested a review from a team as a code owner December 4, 2024 11:56
@gusfcarvalho gusfcarvalho requested a review from moolen December 4, 2024 11:56
Skarlso
Skarlso previously approved these changes Dec 4, 2024
Signed-off-by: Gustavo Carvalho <gusfcarvalho@gmail.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Dec 4, 2024

@Skarlso Skarlso merged commit bd35116 into main Dec 4, 2024
@Skarlso Skarlso deleted the gc/chore/deprecate-olm branch December 4, 2024 16:48
framsouza added a commit to framsouza/external-secrets that referenced this pull request Aug 17, 2025
Replace error suppression approach with proper finalizer-based cleanup
to prevent race conditions when namespaces are deleted.

Changes:
- Add finalizers to ClusterExternalSecret and ExternalSecret controllers
- Implement proper cleanup order during deletion
- Add namespace deletion checks before creating ExternalSecrets
- Remove namespace finalizers during CES cleanup
- Handle managed secret cleanup based on DeletionPolicy

This ensures resources are cleaned up in the correct order and prevents
errors when namespaces are terminating.

Fixes external-secrets#4175

Signed-off-by: framsouza <fram.souza14@gmail.com>
jakobmoellerdev added a commit that referenced this pull request Sep 5, 2025
* fix: handle namespace deletion race conditions with finalizers

Replace error suppression approach with proper finalizer-based cleanup
to prevent race conditions when namespaces are deleted.

Changes:
- Add finalizers to ClusterExternalSecret and ExternalSecret controllers
- Implement proper cleanup order during deletion
- Add namespace deletion checks before creating ExternalSecrets
- Remove namespace finalizers during CES cleanup
- Handle managed secret cleanup based on DeletionPolicy

This ensures resources are cleaned up in the correct order and prevents
errors when namespaces are terminating.

Fixes #4175

Signed-off-by: framsouza <fram.souza14@gmail.com>

* fix: resolve CI failures - lint and race conditions

- Fixed godot lint errors by adding periods to comment endings
- Fixed namespace update race conditions by:
  - Fetching latest namespace before updates to avoid conflicts
  - Handling IsConflict errors with immediate requeue
  - Making test retry on conflict when updating namespaces
- All tests now pass locally

Signed-off-by: framsouza <fram.souza14@gmail.com>

* Address review feedback and improve error handling

Based on reviewer feedback, this commit improves the code quality:

Controller behavior fixes:
- Return immediately after Update() calls to let Kubernetes trigger
  the next reconciliation naturally rather than continuing
- Replace deprecated Requeue field with RequeueAfter: 0 for
  immediate reconciliation on conflicts

Error handling improvements:
- Properly return errors from cleanup functions instead of just
  logging them
- Aggregate multiple errors during cleanup and return them all
- Add clear comments explaining the namespace finalizer purpose

The namespace finalizer prevents race conditions when namespaces
are deleted while ClusterExternalSecrets still have resources in
them. Each CES gets its own finalizer to track its resources
independently.

Signed-off-by: framsouza <fram.souza14@gmail.com>

* fix: add namespace update permissions for ClusterExternalSecret controller

The controller needs permission to update namespaces when managing finalizers
to prevent race conditions during namespace deletion.

Signed-off-by: framsouza <fram.souza14@gmail.com>

* fix: resolve unused parameter linter error

The log parameter in cleanupExternalSecrets was not being used,
causing linter failures in CI. Changed to underscore to indicate
intentionally unused parameter.

Signed-off-by: framsouza <fram.souza14@gmail.com>

* refactor: reduce cognitive complexity in cleanup methods

Split cleanupExternalSecrets into smaller functions to improve readability.
Extracted namespace and finalizer operations into separate helper methods.
Reduces SonarCloud complexity score from 30 to under 10.

Signed-off-by: framsouza <fram.souza14@gmail.com>

* fix: resolve golangci-lint errors

- Add periods to end of comment lines to fix godot linter errors
- Combine repeated parameter types to fix gocritic paramTypeCombine warnings
- Updated function signatures to use combined type declarations (e.g., namespaceName, finalizer string)

Signed-off-by: framsouza <fram.souza14@gmail.com>

* refactor: improve finalizer handling and error aggregation

- Always attempt cleanup on deletion to handle edge cases
- Only call Update() when finalizers actually change
- Use errors.Join() instead of error slices
- Return error instead of RequeueAfter:0 for conflicts

Addresses review feedback from jakobmoellerdev.

Signed-off-by: framsouza <francismara.souza@elastic.co>

* docs(helm): Add detailed comment for processClusterExternalSecret flag

Clarifies that enabling processClusterExternalSecret adds update/patch
permissions on namespaces to handle finalizers for proper cleanup during
namespace deletion, preventing race conditions with ExternalSecrets.

Signed-off-by: framsouza <francismara.souza@elastic.co>

* docs(helm): Update generated README with processClusterExternalSecret documentation

The helm chart README is auto-generated from values.yaml comments.
This commit includes the generated documentation for the
processClusterExternalSecret flag.

Signed-off-by: framsouza <francismara.souza@elastic.co>

---------

Signed-off-by: framsouza <fram.souza14@gmail.com>
Signed-off-by: framsouza <francismara.souza@elastic.co>
Co-authored-by: Moritz Johner <moolen@users.noreply.github.com>
Co-authored-by: Jakob Möller <jakob.moeller@sap.com>
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.

3 participants