Skip to content

fix: Set namespace for AppProjects and repositories #571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+417 −0
Open

fix: Set namespace for AppProjects and repositories #571

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0805ec9
fix: Set namespace for AppProjects and repositories
jannfis Sep 17, 2025
574b3eb
Set agent namespace earlier
jannfis Oct 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions agent/inbound.go
View file
Open in desktop
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.

argocd-agent/agent/inbound.go

Line 199 in a2a7a63

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

argocd-agent/agent/inbound.go

Line 284 in a2a7a63

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

Copy link
Contributor

@gnunn1 gnunn1 Oct 1, 2025 •
edited
Loading

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.

Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,8 @@ func (a *Agent) createAppProject(incoming *v1alpha1.AppProject) (*v1alpha1.AppPr
return nil, event.NewEventDiscardedErr("cannot create appproject: agent is not in managed mode")
}

incoming.SetNamespace(a.namespace)

// If we receive a new AppProject event for an AppProject we already manage, it usually
// means that we're out-of-sync from the control plane.
if a.projectManager.IsManaged(incoming.Name) {
Expand Down Expand Up @@ -597,6 +599,8 @@ func (a *Agent) updateAppProject(incoming *v1alpha1.AppProject) (*v1alpha1.AppPr
"resourceVersion": incoming.ResourceVersion,
})

incoming.SetNamespace(a.namespace)

if !a.projectManager.IsManaged(incoming.Name) {
logCtx.Trace("AppProject is not managed on this agent. Creating the new AppProject")
return a.createAppProject(incoming)
Expand All @@ -622,6 +626,8 @@ func (a *Agent) deleteAppProject(project *v1alpha1.AppProject) error {
"appProject": project.Name,
})

project.SetNamespace(a.namespace)

// If we receive an update appproject event for an AppProject we don't know about yet it
// means that we're out-of-sync from the control plane.
//
Expand Down Expand Up @@ -655,6 +661,8 @@ func (a *Agent) createRepository(incoming *corev1.Secret) (*corev1.Secret, error
"repo": incoming.Name,
})

incoming.SetNamespace(a.namespace)

// In modes other than "managed", we don't process new repository events
// that are incoming.
if a.mode.IsAutonomous() {
Expand Down Expand Up @@ -695,6 +703,8 @@ func (a *Agent) updateRepository(incoming *corev1.Secret) (*corev1.Secret, error
"resourceVersion": incoming.ResourceVersion,
})

incoming.SetNamespace(a.namespace)

if !a.repoManager.IsManaged(incoming.Name) {
logCtx.Trace("Repository is not managed on this agent. Creating the new Repository")
return a.createRepository(incoming)
Expand Down
179 changes: 179 additions & 0 deletions agent/inbound_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,185 @@ func Test_UpdateAppProject(t *testing.T) {
require.Empty(t, napp.OwnerReferences, "OwnerReferences should not be applied on managed app project")
})

// Namespace handling tests
t.Run("CreateAppProject sets correct namespace", func(t *testing.T) {
agentNamespace := "agent-namespace"
a, _ := newAgent(t)
a.namespace = agentNamespace
a.mode = types.AgentModeManaged

be := backend_mocks.NewAppProject(t)
var err error
a.projectManager, err = appproject.NewAppProjectManager(be, agentNamespace, appproject.WithAllowUpsert(true))
require.NoError(t, err)

project := &v1alpha1.AppProject{
ObjectMeta: v1.ObjectMeta{
Name: "test-project",
Namespace: "wrong-namespace", // This should be overridden
},
Spec: v1alpha1.AppProjectSpec{
SourceNamespaces: []string{"default"},
},
}

// Mock the backend Create method and capture the project passed to it
var capturedProject *v1alpha1.AppProject
createMock := be.On("Create", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
capturedProject = args[1].(*v1alpha1.AppProject)
}).Return(&v1alpha1.AppProject{}, nil)
defer createMock.Unset()

_, err = a.createAppProject(project)
require.NoError(t, err)

// Verify that the namespace was set correctly
require.NotNil(t, capturedProject, "Project should have been passed to backend")
assert.Equal(t, agentNamespace, capturedProject.Namespace, "Project namespace should be set to agent namespace")
assert.Equal(t, "test-project", capturedProject.Name, "Project name should remain unchanged")
})

t.Run("UpdateAppProject sets correct namespace", func(t *testing.T) {
agentNamespace := "agent-namespace"
a, _ := newAgent(t)
a.namespace = agentNamespace
a.mode = types.AgentModeManaged

be := backend_mocks.NewAppProject(t)
var err error
a.projectManager, err = appproject.NewAppProjectManager(be, agentNamespace,
appproject.WithAllowUpsert(true),
appproject.WithMode(manager.ManagerModeManaged),
appproject.WithRole(manager.ManagerRoleAgent))
require.NoError(t, err)

project := &v1alpha1.AppProject{
ObjectMeta: v1.ObjectMeta{
Name: "test-project",
Namespace: "wrong-namespace", // This should be overridden
ResourceVersion: "12345",
},
Spec: v1alpha1.AppProjectSpec{
SourceNamespaces: []string{"default"},
},
}

// Set up project as managed
a.projectManager.Manage(project.Name)
defer a.projectManager.Unmanage(project.Name)

// Mock the backend methods
getMock := be.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(&v1alpha1.AppProject{}, nil)
defer getMock.Unset()

supportsPatchMock := be.On("SupportsPatch").Return(true)
defer supportsPatchMock.Unset()

// Capture the namespace passed to Patch method
var capturedNamespace string
patchMock := be.On("Patch", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
capturedNamespace = args[2].(string)
}).Return(&v1alpha1.AppProject{}, nil)
defer patchMock.Unset()

_, err = a.updateAppProject(project)
require.NoError(t, err)

// Verify that the namespace was set correctly in the patch call
assert.Equal(t, agentNamespace, capturedNamespace, "Patch should use agent namespace")
})

t.Run("DeleteAppProject sets correct namespace", func(t *testing.T) {
agentNamespace := "agent-namespace"
a, _ := newAgent(t)
a.namespace = agentNamespace
a.mode = types.AgentModeManaged

be := backend_mocks.NewAppProject(t)
var err error
a.projectManager, err = appproject.NewAppProjectManager(be, agentNamespace, appproject.WithAllowUpsert(true))
require.NoError(t, err)

project := &v1alpha1.AppProject{
ObjectMeta: v1.ObjectMeta{
Name: "test-project",
Namespace: "wrong-namespace", // This should be overridden
},
Spec: v1alpha1.AppProjectSpec{
SourceNamespaces: []string{"default"},
},
}

// Set up project as managed
a.projectManager.Manage(project.Name)

// Mock the backend Delete method and capture the namespace passed to it
var capturedNamespace string
deleteMock := be.On("Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
capturedNamespace = args[2].(string)
}).Return(nil)
defer deleteMock.Unset()

err = a.deleteAppProject(project)
require.NoError(t, err)

// Verify that the namespace was set correctly
assert.Equal(t, agentNamespace, capturedNamespace, "Delete should use agent namespace")
})

t.Run("AppProject operations with custom agent namespace", func(t *testing.T) {
customAgentNamespace := "custom-agent-ns"
a, _ := newAgent(t)
a.namespace = customAgentNamespace
a.mode = types.AgentModeManaged

be := backend_mocks.NewAppProject(t)
var err error
a.projectManager, err = appproject.NewAppProjectManager(be, customAgentNamespace, appproject.WithAllowUpsert(true))
require.NoError(t, err)

project := &v1alpha1.AppProject{
ObjectMeta: v1.ObjectMeta{
Name: "test-project",
Namespace: "principal-namespace", // Different from agent
},
Spec: v1alpha1.AppProjectSpec{
SourceNamespaces: []string{"default"},
},
}

// Test Create
var capturedCreateProject *v1alpha1.AppProject
createMock := be.On("Create", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
capturedCreateProject = args[1].(*v1alpha1.AppProject)
}).Return(&v1alpha1.AppProject{}, nil)

_, err = a.createAppProject(project)
require.NoError(t, err)
createMock.Unset()

// Verify that the namespace was overridden to agent namespace
require.NotNil(t, capturedCreateProject, "Project should have been passed to backend")
assert.Equal(t, customAgentNamespace, capturedCreateProject.Namespace, "Project namespace should be set to custom agent namespace")
assert.NotEqual(t, "principal-namespace", capturedCreateProject.Namespace, "Project namespace should not remain as principal namespace")

// Test Delete
a.projectManager.Manage(project.Name)

var capturedDeleteNamespace string
deleteMock := be.On("Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
capturedDeleteNamespace = args[2].(string)
}).Return(nil)

err = a.deleteAppProject(project)
require.NoError(t, err)
deleteMock.Unset()

// Verify that the namespace was overridden to agent namespace
assert.Equal(t, customAgentNamespace, capturedDeleteNamespace, "Delete should use custom agent namespace")
assert.NotEqual(t, "principal-namespace", capturedDeleteNamespace, "Delete should not use principal namespace")
})

}

func Test_ProcessIncomingRepositoryWithUIDMismatch(t *testing.T) {
Expand Down
5 changes: 5 additions & 0 deletions principal/event.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ func (s *Server) processAppProjectEvent(ctx context.Context, agentName string, e
// AppProject creation event will only be processed in autonomous mode
case event.Create.String():
if agentMode.IsAutonomous() {
incoming.SetNamespace(s.namespace)
_, err := s.projectManager.Create(ctx, incoming)
if err != nil {
return fmt.Errorf("could not create app-project %s: %w", incoming.Name, err)
Expand All @@ -335,6 +336,8 @@ func (s *Server) processAppProjectEvent(ctx context.Context, agentName string, e
return event.NewEventNotAllowedErr("event type not allowed when mode is not autonomous")
}

incoming.SetNamespace(s.namespace)

_, err := s.projectManager.UpdateAppProject(ctx, incoming)
if err != nil {
return fmt.Errorf("could not update app-project %s: %w", incoming.Name, err)
Expand All @@ -346,6 +349,8 @@ func (s *Server) processAppProjectEvent(ctx context.Context, agentName string, e
return event.NewEventNotAllowedErr("event type not allowed when mode is not autonomous")
}

incoming.SetNamespace(s.namespace)

deletionPropagation := backend.DeletePropagationForeground
err := s.projectManager.Delete(ctx, incoming, &deletionPropagation)
if err != nil {
Expand Down
Loading
Loading