Skip to content

Conversation

jannfis
Copy link
Collaborator

@jannfis jannfis commented Sep 17, 2025

What does this PR do / why we need it:

The AppProject and Repository Credentials management of both agent and principal do not set the namespace properly. Thus, syncing those resources works only when principal and agent are running in the same namespace (i.e. "argocd" or similar).

This PR sets the namespace explicitly to the principal's or agent's namespace before trying to create/update/delete said resources.

Which issue(s) this PR fixes:

Fixes #570

How to test changes / Special notes to the reviewer:

Checklist

  • Documentation update is required by this PR (and has been updated) OR no documentation update is required.

@codecov-commenter
Copy link

codecov-commenter commented Sep 17, 2025

Codecov Report

❌ Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.83%. Comparing base (a2a7a63) to head (574b3eb).
⚠️ Report is 20 commits behind head on main.

Files with missing lines Patch % Lines
principal/event.go 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #571      +/-   ##
==========================================
+ Coverage   44.52%   45.83%   +1.30%     
==========================================
  Files          90       90              
  Lines       11909    12053     +144     
==========================================
+ Hits         5302     5524     +222     
+ Misses       6174     6083      -91     
- Partials      433      446      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Should we also update the namespace before comparing the source UID? Otherwise it may try to fetch the resource from the agent's namespace.

exists, sourceUIDMatch, err := a.projectManager.CompareSourceUID(a.context, incomingAppProject)

exists, sourceUIDMatch, err = a.repoManager.CompareSourceUID(a.context, incomingRepo)

Copy link
Contributor

@gnunn1 gnunn1 Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm testing this PR out and I wonder if I'm hitting this issue, I have the principal in openshift-gitops-principal and the agent in openshift-gitops on the same cluster. The AppProject doesn't get copied over to the Agents namespace and I see this message in the agent log:

time="2025-10-01T22:10:24Z" level=error msg="Unable to process incoming event" clientAddr="172.31.75.84:443" direction=Recv error="failed to validate source UID of appProject: appprojects.argoproj.io \"local-cluster\" is forbidden: User \"system:serviceaccount:openshift-gitops:argocd-agent-agent\" cannot get resource \"appprojects\" in API group \"argoproj.io\" in the namespace \"openshift-gitops-principal\"" event_id=local-cluster_7a8ac8fd-5fe9-4dc6-823d-f37c783b77ea_143200 module=StreamEvent resource_id=local-cluster_7a8ac8fd-5fe9-4dc6-823d-f37c783b77ea type=io.argoproj.argocd-agent.event.spec-update

I did build and deployed the agent from the branch with the fix.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

system:serviceaccount:openshift-gitops:argocd-agent-agent" cannot get resource "appprojects" in API group "argoproj.io" in the namespace "openshift-gitops-principal\

I think it's the same issue. The agent is trying to fetch the AppProject from the principal namespace instead of it's own agent namespace.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Now thinking about it, I believe the SetNamespace call makes most sense early in the processIncoming... functions, instead of in the create/delete/update functions.

I'll make the change and test it thoroughly.

Copy link
Collaborator

@chetan-rns chetan-rns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Added a small question. Everything else looks good to me.

@jannfis
Copy link
Collaborator Author

jannfis commented Oct 8, 2025

@gnunn1 Would you be able to re-test this PR to see if the recent changes fixed the issues you were seeing?

@gnunn1
Copy link
Contributor

gnunn1 commented Oct 8, 2025

I can but not until next week if that's OK.

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.

AppProject namespace not updated by Agent
4 participants