Skip to content
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.

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

Multiple clusters with the same URL and different token may not be synchronized #9606

Open
3 tasks done
shmurata opened this issue Jun 8, 2022 · 2 comments · May be fixed by #10897
Open
3 tasks done

Multiple clusters with the same URL and different token may not be synchronized #9606

shmurata opened this issue Jun 8, 2022 · 2 comments · May be fixed by #10897
Labels
bug Something isn't working

Comments

@shmurata
Copy link

shmurata commented Jun 8, 2022

Checklist:

  • I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
  • I've included steps to reproduce the bug.
  • I've pasted the output of argocd version.

Describe the bug

Multiple clusters with the same URL and different token may not be synchronized.

App synchronization may fail if there are multiple clusters with the same URL and different token.

I don't think there is a way to set this up with the current ArgoCD in the case where there are not permissions for all namespaces in the cluster to which you are deploying, and you want to use different ServiceAccounts for each namespace for two or more namespaces.

To Reproduce

  1. Register two or more clusters with the following conditions according to https://argo-cd.readthedocs.io/en/stable/operator-manual/declarative-setup/#clusters
    • Same server
    • Different names
    • Different namespace
    • Different bearerToken
      • Each token is only authorized to deploy to the specified namespace.
  2. Specify the clusters registered above in Application.Spec.Destination.Name.
  3. Deploy the Application.

Deployment may succeed or fail here.
If it fails, it is an authorization error and a different namespace token may be used.

Expected behavior

Expect deployments to be made using the settings for the cluster with the specified name each time.

Perhaps the cluster information retrieved in this line is being retrieved by URL only, causing the results to vary depending on the order of the clusters in the ArgoDB information.
https://github.com/argoproj/argo-cd/blob/v2.3.4/controller/sync.go#L125

This problem has been fixed by changing this part to get by Name.

Screenshots

Version

Paste the output from `argocd version` here.
2.3.4

Logs

Paste any relevant application logs here.
@shmurata shmurata added the bug Something isn't working label Jun 8, 2022
@pnieweglowski
Copy link

@shmurata would you share, what changes have you done in sync.go file?

@shmurata
Copy link
Author

@pnieweglowski I got around this by changing to get clusters by name as follows.
I don't know if this is the right way to do it because I don't know all the affected codes.

```diff diff --git a/controller/sync.go b/controller/sync.go index a50aa1c0e..4dcdadce2 100644 --- a/controller/sync.go +++ b/controller/sync.go @@ -122,11 +122,14 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha return }
  • clst, err := m.db.GetCluster(context.Background(), app.Spec.Destination.Server)
  • clst, err := m.db.GetClusterByName(context.Background(), app.Spec.Destination.Name)
    if err != nil {
  •   state.Phase = common.OperationError
    
  •   state.Message = err.Error()
    
  •   return
    
  •   clst, err = m.db.GetCluster(context.Background(), app.Spec.Destination.Server)
    
  •   if err != nil {
    
  •   	state.Phase = common.OperationError
    
  •   	state.Message = err.Error()
    
  •   	return
    
  •   }
    

    }

    rawConfig := clst.RawRestConfig()
    diff --git a/util/db/cluster.go b/util/db/cluster.go
    index 64e8db358..28bdc0a0b 100644
    --- a/util/db/cluster.go
    +++ b/util/db/cluster.go
    @@ -211,6 +211,22 @@ func (db *db) GetCluster(_ context.Context, server string) (*appv1.Cluster, erro
    return nil, status.Errorf(codes.NotFound, "cluster %q not found", server)
    }

+// GetClusterByName returns a cluster from a query
+func (db *db) GetClusterByName(_ context.Context, name string) (*appv1.Cluster, error) {

  • informer, err := db.settingsMgr.GetSecretsInformer()
  • if err != nil {
  •   return nil, err
    
  • }
  • res, err := informer.GetIndexer().ByIndex(settings.ByClusterNameIndexer, name)
  • if err != nil {
  •   return nil, err
    
  • }
  • if len(res) > 0 {
  •   return secretToCluster(res[0].(*apiv1.Secret))
    
  • }
  • return nil, status.Errorf(codes.NotFound, "cluster %q not found", name)
    +}

// GetProjectClusters return project scoped clusters by given project name
func (db *db) GetProjectClusters(ctx context.Context, project string) ([]*appv1.Cluster, error) {
informer, err := db.settingsMgr.GetSecretsInformer()
diff --git a/util/db/db.go b/util/db/db.go
index 05ae38e75..e803233df 100644
--- a/util/db/db.go
+++ b/util/db/db.go
@@ -29,6 +29,8 @@ type ArgoDB interface {
handleDeleteEvent func(clusterServer string)) error
// GetCluster returns a cluster by given server url
GetCluster(ctx context.Context, server string) (*appv1.Cluster, error)

  • // GetClusterByName returns a cluster by given server name
  • GetClusterByName(ctx context.Context, name string) (*appv1.Cluster, error)
    // GetClusterServersByName returns a cluster server urls by given cluster name
    GetClusterServersByName(ctx context.Context, name string) ([]string, error)
    // GetProjectClusters return project scoped clusters by given project name
</details>

@denysvitali denysvitali linked a pull request Oct 11, 2022 that will close this issue
10 tasks
denysvitali pushed a commit to swisscom/argo-cd that referenced this issue Nov 30, 2022
This is the first commit towards addressing argoproj#9606

Signed-off-by: Denys Vitali <denys.vitali@swisscom.com>
denysvitali pushed a commit to swisscom/argo-cd that referenced this issue Nov 30, 2022
This is the first commit towards addressing argoproj#9606

Signed-off-by: Denys Vitali <denys.vitali@swisscom.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants