feature/mv-kubeconfig #638
Conversation
dosubot
bot
added
the
size:L
WalkthroughThis pull request updates the tenant application version and its associated version map, adds a new HelmRelease resource (conditioned on OIDC configuration) in the tenant templates, and enhances the RBAC permissions by adding the "infos" resource to the admin role. In addition, it introduces a new "info" Helm chart package with its own configuration files, documentation, and related templates. Finally, new application entries for "Info" are added to system components in both the API and dashboard configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant K8s as Kubernetes Cluster
participant Config as OIDC ConfigMap ("cozystack")
participant Helm as Helm Controller
participant Repo as HelmRepository (cozystack-extra)
participant Chart as Info Chart
K8s->>Config: Lookup "cozystack" config
alt OIDC enabled
Config-->>K8s: Return OIDC configuration
K8s->>Helm: Create HelmRelease resource for "info"
Helm->>Repo: Fetch chart "info"
Repo-->>Helm: Deliver Helm chart
Helm->>K8s: Deploy "info" application
else OIDC disabled
Config-->>K8s: No OIDC configuration found
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/extra/info/README.md (1)
1-2: Add description of the Info package.The README should start with a brief description of what the Info package does and its purpose.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/extra/info/logos/info.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
packages/apps/tenant/Chart.yaml(1 hunks)packages/apps/tenant/templates/info.yaml(1 hunks)packages/apps/tenant/templates/tenant.yaml(1 hunks)packages/apps/versions_map(1 hunks)packages/extra/info/.helmignore(1 hunks)packages/extra/info/Chart.yaml(1 hunks)packages/extra/info/Makefile(1 hunks)packages/extra/info/README.md(1 hunks)packages/extra/info/templates/dashboard-resourcemap.yaml(1 hunks)packages/extra/info/templates/kubeconfig.yaml(2 hunks)packages/extra/info/values.schema.json(1 hunks)packages/extra/versions_map(1 hunks)packages/system/cozystack-api/templates/configmap.yaml(1 hunks)packages/system/dashboard/values.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- packages/extra/info/values.schema.json
- packages/extra/info/.helmignore
- packages/apps/tenant/Chart.yaml
- packages/extra/info/Makefile
- packages/extra/info/Chart.yaml
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
packages/extra/info/README.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🪛 YAMLlint (1.35.1)
packages/apps/tenant/templates/info.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (11)
packages/apps/versions_map (2)
105-106: LGTM! Version increments follow semantic versioning.The version increments from 1.6.8 to 1.7.0 follow semantic versioning principles, with proper commit hash references.
1-130: Inconsistency between code and AI summary.The AI summary mentions a new
infopackage with version 1.0.0, but this package is not present in the actual code changes.Likely an incorrect or invalid review comment.
packages/extra/versions_map (1)
10-10: LGTM!The new entry for the
infopackage follows the established format and versioning pattern.packages/extra/info/templates/dashboard-resourcemap.yaml (1)
1-12: LGTM! Well-scoped RBAC permissions.The Role is correctly configured with:
- Minimal read-only permissions
- Specific resource name pattern using Release.Namespace
- Proper scoping to only access kubeconfig secrets
packages/apps/tenant/templates/info.yaml (1)
1-3:❓ Verification inconclusive
Verify OIDC configuration lookup.
The OIDC configuration lookup should handle cases where:
- The ConfigMap doesn't exist
- The 'oidc-enabled' key is missing
🏁 Script executed:
#!/bin/bash # Description: Check if the ConfigMap and key exist in other namespaces # Test: Search for ConfigMap definition rg -A 5 'kind:\s+ConfigMap.*name:\s+cozystack' # Test: Search for oidc-enabled key usage rg 'oidc-enabled'Length of output: 614
Action: Confirm OIDC Lookup Handling of Missing ConfigMap/Key
The lookup in
packages/apps/tenant/templates/info.yamlis implemented consistently with other parts of the codebase. However, please verify that it safely handles cases where:
- The ConfigMap does not exist
- The “oidc-enabled” key is absent
If these cases are possible in your environment, consider enhancing the template with an explicit nil (or existence) check or a default value (for example, using a construct like
if and $cozyConfig $cozyConfig.data (index $cozyConfig.data "oidc-enabled")) to avoid potential runtime issues. Otherwise, if the current behavior is intentional and the ConfigMap is guaranteed to exist, please confirm.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/extra/info/templates/kubeconfig.yaml (3)
18-18: Metadata Name Update: Using Release NamespaceChanging the Secret’s metadata name from a tenant-derived value to use
{{ .Release.Namespace }}standardizes resource naming based on the deployment namespace. This improves consistency for multi-tenant installations.
30-30: Context Namespace AlignmentThe context’s namespace field is now set to
{{ .Release.Namespace }}. This update aligns the kubeconfig context with the release namespace, ensuring that the generated configuration accurately reflects the deployment environment.
32-33: Context Name and Current-Context ConsistencyUpdating both the context
nameandcurrent-contextto{{ .Release.Namespace }}reinforces consistency across the kubeconfig file. This change helps avoid misconfiguration by tying all aspects of the context to the release’s namespace.packages/system/cozystack-api/templates/configmap.yaml (1)
317-331: New Application Entry: InfoA new application entry for Info has been added with the following attributes:
•kind: Info
•singular: info
•plural: infosThe release section (with an empty prefix and UI label) and the chart reference to the
infochart in thecozystack-extraHelm repository are consistent with other entries. Please verify that related components (e.g., the HelmRelease and dashboard configurations) correctly refer to these new values.packages/apps/tenant/templates/tenant.yaml (1)
275-283: Expanded Admin Role RBAC PermissionsThe admin Role has been extended to include the new
infosresource under theapps.cozystack.ioAPI group with full permissions (get, list, watch, create, update, patch, delete). This update enables administrators to manage the new Info resource. Double-check that this permission expansion aligns with your security requirements.packages/system/dashboard/values.yaml (1)
364-378: New Dashboard Application Configuration: InfoA new entry for the Info application is introduced in the
pluginConfigunderkubeappsapis. The configuration correctly defines:
•kind: Info
•singular: info
•plural: infosThe release settings (empty prefix, UI label, and chart reference to
infofrom thecozystack-extraHelm repository) mirror similar configurations in other parts of the system. This addition should integrate smoothly within the dashboard’s plugin structure.
| ### Kubeconfig for tenant | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix heading structure and add content.
The heading structure should be fixed and content should be added:
- Use
## Kubeconfig for tenant(h2) instead of h3 - Add instructions on how to obtain and configure the kubeconfig for a tenant
-### Kubeconfig for tenant
+## Kubeconfig for tenant
+
+Instructions on how to obtain and configure the kubeconfig for a tenant:
+1. ...
+2. ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Kubeconfig for tenant | |
| ## Kubeconfig for tenant | |
| Instructions on how to obtain and configure the kubeconfig for a tenant: | |
| 1. ... | |
| 2. ... |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
| ### Kubelogin | ||
|
|
||
| For using kubeconfig need install kubelogin. | ||
|
|
||
| ```bash | ||
| # Homebrew (macOS and Linux) | ||
| brew install int128/kubelogin/kubelogin | ||
|
|
||
| # Krew (macOS, Linux, Windows and ARM) | ||
| kubectl krew install oidc-login | ||
|
|
||
| # Chocolatey (Windows) | ||
| choco install kubelogin | ||
| ``` |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add version requirements for kubelogin.
The installation instructions should specify:
- Minimum required version of kubelogin
- Any compatibility requirements with the cluster version
🌐 Web query:
What is the latest version of kubelogin and its compatibility requirements?
💡 Result:
The latest version of kubelogin is v0.1.7, as indicated by its most recent GitHub release[10]. Here's a breakdown of its compatibility requirements:
Compatibility Requirements
-
Kubernetes Version Support
- Required for Azure Active Directory (AAD)-enabled clusters in Kubernetes 1.24+ due to the removal of built-in AAD authentication in
kubectlstarting with Kubernetes 1.26[2][7][8]. - Works with all supported Microsoft Entra (formerly AAD) authentication methods in Azure Kubernetes Service (AKS), including Azure CLI, device code, and interactive browser flows[4].
- Required for Azure Active Directory (AAD)-enabled clusters in Kubernetes 1.24+ due to the removal of built-in AAD authentication in
-
Operating Systems
- Compatible with Linux, macOS, and Windows[2][7][10].
-
Integration Requirements
- Must be installed on systems where tasks like
KubernetesManifest@1,HelmDeploy@0, orAzureFunctionOnKubernetes@1are executed in CI/CD pipelines[1]. - Requires a valid kubeconfig file modified using
kubelogin convert-kubeconfigto enable AAD authentication[2][4].
- Must be installed on systems where tasks like
-
Authentication Dependencies
- Relies on Azure CLI (
az) for token generation in most user-authentication scenarios[2][4]. - Uses Microsoft Entra applications:
- Server application ID:
6dae42f8-4368-4678-94ff-3960e28e3630 - Client application ID:
80faf920-1908-4b52-b5ef-a8e7bedfc67a[4].
- Server application ID:
- Relies on Azure CLI (
Key Notes
- Earlier Kubernetes versions (≤1.22) had built-in AAD support in
kubectl, but this was deprecated and removed in later releases[2][7]. - Installation methods include direct downloads from GitHub, package managers like Homebrew/APT/YUM[7][10], or integration into Azure DevOps pipelines via
KubeloginInstaller@0[1].
Citations:
- 1: https://learn.microsoft.com/en-us/azure/devops/pipelines/tasks/reference/kubelogin-installer-v0?view=azure-pipelines
- 2: https://aptakube.com/blog/kubelogin-is-not-installed-which-is-required-to-connect-to-aad-enabled-cluster
- 3: https://docs.newrelic.com/docs/kubernetes-pixie/kubernetes-integration/get-started/kubernetes-integration-compatibility-requirements/
- 4: https://learn.microsoft.com/en-us/azure/aks/kubelogin-authentication
- 5: https://github.com/int128/kubelogin/blob/master/docs/setup.md
- 6: https://kubernetes.io/docs/tasks/tools/install-kubectl-linux/
- 7: https://docs.wakemeops.com/packages/kubelogin/
- 8: https://azure.github.io/kubelogin/
- 9: https://kubernetes.io/docs/tasks/tools/install-kubectl-windows/
- 10: https://github.com/azure/kubelogin/releases
Action: Update the README Installation Instructions to Specify Kubelogin Version and Compatibility
Please update the instructions in packages/extra/info/README.md (lines 5–18) with the following details:
- Minimum Required Version: Indicate that kubelogin's minimum required version is v0.1.7.
- Compatibility Requirements:
- Note that for AAD-enabled clusters, kubelogin is intended for Kubernetes 1.24+ (reflecting the removal of built-in AAD support in later Kubernetes releases).
- Optionally, clarify the supported operating systems (Linux, macOS, Windows) and reference the need for valid authentication configurations (e.g., kubeconfig conversion and Azure CLI integration).
Updating these points will help users ensure they have the correct versions and configurations, reducing potential integration issues.
| kind: HelmRepository | ||
| name: cozystack-extra | ||
| namespace: cozy-public | ||
| version: "*" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using wildcard version selector.
Using "*" for version could lead to unexpected upgrades. Consider:
- Using a specific version
- Using a version range with upper bound
- version: "*"
+ version: "1.0.0"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| version: "*" | |
| version: "1.0.0" |
2 participants
Summary by CodeRabbit
New Features
Documentation
Chores