diff --git a/manager/controlapi/cluster.go b/manager/controlapi/cluster.go index 9832272a3f..7e9dea2ce5 100644 --- a/manager/controlapi/cluster.go +++ b/manager/controlapi/cluster.go @@ -247,6 +247,11 @@ func redactClusters(clusters []*api.Cluster) []*api.Cluster { // Do not copy secret keys redactedSpec := cluster.Spec.Copy() redactedSpec.CAConfig.SigningCAKey = nil + // the cert is not a secret, but if API users get the cluster spec and then update, + // then because the cert is included but not the key, the user can get update errors + // or unintended consequences (such as telling swarm to forget about the key so long + // as there is a corresponding external CA) + redactedSpec.CAConfig.SigningCACert = nil redactedRootCA := cluster.RootCA.Copy() redactedRootCA.CAKey = nil diff --git a/manager/controlapi/cluster_test.go b/manager/controlapi/cluster_test.go index 372165dfe6..7df369b4ea 100644 --- a/manager/controlapi/cluster_test.go +++ b/manager/controlapi/cluster_test.go @@ -440,8 +440,8 @@ func TestUpdateClusterRotateUnlockKey(t *testing.T) { } // root rotation tests have already been covered by ca_rotation_test.go - this test only makes sure that the function tested in those -// tests is actually called by `UpdateCluster`, and that the results of GetCluster and ListCluster have the rotation root CA key -// redacted +// tests is actually called by `UpdateCluster`, and that the results of GetCluster and ListCluster have the CA keys +// and the spec key and cert redacted func TestUpdateClusterRootRotation(t *testing.T) { ts := newTestServer(t) defer ts.Stop() @@ -464,26 +464,57 @@ func TestUpdateClusterRootRotation(t *testing.T) { }) require.NoError(t, err) - var gottenClusters []*api.Cluster + checkCluster := func() *api.Cluster { + response, err = ts.Client.GetCluster(context.Background(), &api.GetClusterRequest{ClusterID: cluster.ID}) + require.NoError(t, err) + require.NotNil(t, response.Cluster) - response, err = ts.Client.GetCluster(context.Background(), &api.GetClusterRequest{ClusterID: cluster.ID}) - require.NoError(t, err) - require.NotNil(t, response.Cluster) - gottenClusters = append(gottenClusters, response.Cluster) + listResponse, err := ts.Client.ListClusters(context.Background(), &api.ListClustersRequest{}) + require.NoError(t, err) + require.Len(t, listResponse.Clusters, 1) - listResponse, err := ts.Client.ListClusters(context.Background(), &api.ListClustersRequest{}) - require.NoError(t, err) - require.Len(t, listResponse.Clusters, 1) - gottenClusters = append(gottenClusters, listResponse.Clusters[0]) + require.Equal(t, response.Cluster, listResponse.Clusters[0]) - for _, c := range gottenClusters { + c := response.Cluster require.NotNil(t, c.RootCA.RootRotation) - // check signing key redaction + // check that all keys are redacted, and that the spec signing cert is also redacted (not because + // the cert is a secret, but because that makes it easier to get-and-update) require.Len(t, c.RootCA.CAKey, 0) require.Len(t, c.RootCA.RootRotation.CAKey, 0) require.Len(t, c.Spec.CAConfig.SigningCAKey, 0) + require.Len(t, c.Spec.CAConfig.SigningCACert, 0) + + return c + } + + getUnredactedRootCA := func() (rootCA *api.RootCA) { + ts.Store.View(func(tx store.ReadTx) { + c := store.GetCluster(tx, cluster.ID) + require.NotNil(t, c) + rootCA = &c.RootCA + }) + return } + + cluster = checkCluster() + unredactedRootCA := getUnredactedRootCA() + + // update something else, but make sure this doesn't the root CA rotation doesn't change + updatedSpec = cluster.Spec.Copy() + updatedSpec.CAConfig.NodeCertExpiry = gogotypes.DurationProto(time.Hour) + _, err = ts.Client.UpdateCluster(context.Background(), &api.UpdateClusterRequest{ + ClusterID: cluster.ID, + Spec: updatedSpec, + ClusterVersion: &cluster.Meta.Version, + }) + require.NoError(t, err) + + updatedCluster := checkCluster() + require.NotEqual(t, cluster.Spec.CAConfig.NodeCertExpiry, updatedCluster.Spec.CAConfig.NodeCertExpiry) + updatedUnredactedRootCA := getUnredactedRootCA() + + require.Equal(t, unredactedRootCA, updatedUnredactedRootCA) } func TestListClusters(t *testing.T) {