Skip to content

Conversation

@prakash100198
Copy link
Contributor

@prakash100198 prakash100198 commented May 29, 2023

Description

When data label in argocd-cm is not present then a new chart repo is not getting added and throws nil pointer as we append the chart repo details in argocd-cm, cm.Data is nil and later this map was being used to set a value in one of it's key, hence throwing nil pointer
Fixes #3491

How Has This Been Tested?

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

  • Tested Locally

  • Tested by deploying in my cluster

  • Test Scenarios:-

    1. Devtron installed via helm without argo-cd enabled and remove the data label from argocd-cm to reproduce this issue, now add a chart repo, previously it was throwing nil pointer, after this pr it was working fine
    2. Now installed argo-cd module(this will cover a scenario when a user switches from helm to argocd with empty argocd-cm then any new chart repo addition will not fail), and install the same application with argocd, it was working fine
    3. Go to argo-cd and check if the application is getting registered in argocd or not and also check if sync is happening or not, it was working fine in that case too.
  • Test Scenarios after code refactoring:-
    1. Create new chart repo, check secret is being created or not, and it must not get stored in argocd-cm and check on argocd as well it must get registered there, also try saving same chart url and see if it affect the pre-deployed apps, in this case it's working fine.
    2. Create new chart repo with same name as existing names, it should now show toast as this chart name exist.
    3. After creating new public chart repo, check by deploying few charts from that repo, using gitops/helm both are working fine.
    4. Check for private charts, existing behaviour is expected for private chart, except when new private chart with same name as existing ones is added it must throw toast error.
    5. Update any pre-existing chart, it's updated value should get updated in argocd-cm, and after updating any newly added chart repo it's updated value should get patched in it's corresponding secret.
    6. Try deleting chart repo, for pre-existing chart repos whose entry is in argocd-cm , that entry must get deleted and for newly added chart the corresponding secret should get deleted.
    7. For pre-existing charts deployed, check it's behaviour if a new repo is added with the same url but with diff chart name.
    8. Check an deployed chart behaviour via argocd after deleting the repo entry from argocd-cm and after deleting the secret as well (syncing must happen from github repo in argocd which is working correctly).
    9. Try saving chart repo with uppercase names and lowercase names since secret creation doesn't support uppercase chart names.

Checklist:

  • The title of the PR states what changed and the related issues number (used for the release note).
  • Does this PR requires documentation updates?
  • I've updated documentation as required by this PR.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have tested it for all user roles.
  • I have added all the required unit/api test cases.

Does this PR introduce a user-facing change?


@prakash100198 prakash100198 requested a review from nishant-d May 29, 2023 10:43
@Apollo9999
Copy link

Apollo9999 commented May 29, 2023

To fix the "nil pointer" issue related to the empty data label in ArgoCD ConfigMap (argocd-cm), you can follow these steps:

Step1Locate the ConfigMap definition:
In your Kubernetes cluster, find the ConfigMap resource associated with ArgoCD.
Typically, the ConfigMap name is argocd-cm in the argocd namespace.

Step2 Edit the ConfigMap YAML:
Retrieve the YAML definition of the ConfigMap by running the command: kubectl get configmap argocd-cm -n argocd -o yaml.
Save the output to a local file for editing.

Step3 Update the YAML file:
Open the YAML file in a text editor.
Look for the data section within the ConfigMap definition.
Check if there is an empty data label (e.g., data: {}).
Replace the empty data label with a nil value by removing the data field entirely. It should look like this:
apiVersion: v1
kind: ConfigMap
metadata:
name: argocd-cm
namespace: argocd

Step4 Apply the updated ConfigMap:
Apply the updated YAML file to your Kubernetes cluster by running the command: kubectl apply -f .
By removing the empty data label, you prevent any potential nil pointer errors that could occur due to accessing empty data in the ConfigMap. Keep in mind that this fix assumes that the empty data label is not necessary for your specific use case. If you have any specific data entries required within the ConfigMap, make sure to include them accordingly in the data section.

Please note that modifying the ArgoCD ConfigMap should be done with caution, as it can affect the behavior and configuration of your ArgoCD installation. It's recommended to back up the existing ConfigMap and test the changes in a non-production environment before applying them to a production cluster.

@prakash100198 prakash100198 added the pager-duty Bugs / Issues found while on pager duty label May 29, 2023
@prakash100198 prakash100198 requested a review from nishant-d June 6, 2023 12:06
@prakash100198 prakash100198 requested a review from nishant-d June 7, 2023 13:24
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@prakash100198 prakash100198 merged commit 429d7c8 into main Jun 7, 2023
@prakash100198 prakash100198 deleted the argocd-cm-data-nil-pointer-check-added branch June 7, 2023 14:18
@Apollo9999
Copy link

Thanks a lot

adi6859 pushed a commit that referenced this pull request Jun 16, 2023
* config map data nil pointer fix

* check for nil config map

* creating secret for public chart repo

* delete chart repo based on it's presence in argocd-cm

* code for unique chart name while adding a chart repo

* secret creation doesn't support uppercase letters hence adding checks

* minor change

* minor fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pager-duty Bugs / Issues found while on pager duty

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: not able to save chart repo when argocd-cm has empty data label

5 participants