Refac/organization standardization

[gciezkowski-acc]

Description

I implement new approach to the organization controllers.

What type of PR is this? (check all applicable)

• 🍕 Feature
• 🐛 Bug Fix
• 📝 Documentation Update
• 🎨 Style
• 🧑‍💻 Code Refactor
• 🔥 Performance Improvements
• ✅ Test
• 🤖 Build
• 🔁 CI
• 📦 Chore (Release)
• ⏩ Revert

Related Tickets & Documents

• Related Issue # (issue)
• Closes [REFAC] - Standardise Organisation controllers #583
• Fixes # (issue)

Remove if not applicable

Added tests?

• 👍 yes
• 🙅 no, because they aren't needed
• 🙋 no, because I need help
• Separate ticket for tests # (issue/pr)

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Added to documentation?

• 📜 README.md
• 🤝 Documentation pages updated
• 🙅 no documentation needed
• (if applicable) generated OpenAPI docs for CRD changes

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (no documentation needed)
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[abhijith-darshan]

Is Pending being returned so that the Ready status is not set?

[gciezkowski-acc]

Done, I switched to lifecycle.Success in that case.

[gciezkowski-acc]

I move it to the predicate to filter reconciliation.

[abhijith-darshan]

We should merge all 3 controllers into one since they are acting on the same Custom Resource.

With current approach there will be conflict with DexReconciler and the remaining ones.

if no OIDC config is provided then it will be Pending and the remaining will be in Created state.

There will be potentially an infinite loop where the competing controllers will put Ready and Dex will put it back to Pending

You can merge all 3 functionalities into one Organization Reconciler and the logical order can be -

• Functionality in OrganizationReconciler (Namespace, etc…)
• Functionality in Rbac
• Lastly the DexReconciler and it can exit with a success if there is no OIDC config. You can set a condition to reflect this state.

But since the issue states you don’t need to consolidate at this point we need to bring this up in core dev meeting to check with others

[uwe-mayer]

We should merge all 3 controllers into one since they are acting on the same Custom Resource.

With current approach there will be conflict with DexReconciler and the remaining ones.

if no OIDC config is provided then it will be Pending and the remaining will be in Created state.

There will be potentially an infinite loop where the competing controllers will put Ready and Dex will put it back to Pending

You can merge all 3 functionalities into one Organization Reconciler and the logical order can be -

• Functionality in OrganizationReconciler (Namespace, etc…)
• Functionality in Rbac
• Lastly the DexReconciler and it can exit with a success if there is no OIDC config. You can set a condition to reflect this state.

But since the issue states you don’t need to consolidate at this point we need to bring this up in core dev meeting to check with others

Let's go with a TODO and a Predicate on the SetupWithManager func of the Controller to not reconcile Dex controller on Status changes.