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 1 commit into
base: main
Choose a base branch
from
+396 −0
Open

fix: Set namespace for AppProjects and repositories #571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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)

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