Skip to content

Commit

Permalink
Use ServerCA instead of client secret ca.crt
Browse files Browse the repository at this point in the history
  • Loading branch information
pooneh-m committed May 15, 2020
1 parent da3bc27 commit cec2f2b
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ spec:
properties:
secretName:
type: string
serverCa:
type: string
format: byte
allocationEndpoints:
items:
type: string
Expand Down
3 changes: 3 additions & 0 deletions install/yaml/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,9 @@ spec:
properties:
secretName:
type: string
serverCa:
type: string
format: byte
allocationEndpoints:
items:
type: string
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/multicluster/v1/gameserverallocationpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ type ClusterConnectionInfo struct {
SecretName string `json:"secretName"`
// The cluster namespace from which to allocate gameservers
Namespace string `json:"namespace"`
// The PEM encoded server CA, used by the allocator client to authenticate the remote server.
ServerCA []byte `json:"serverCa,omitempty"`
}

// +genclient
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/multicluster/v1/gameserverallocationpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func TestConnectionInfoIterator(t *testing.T) {
SecretName: "secret-name",
AllocationEndpoints: []string{"allocation-endpoint"},
Namespace: "ns1",
ServerCA: []byte("c2VydmVyQ0E="),
},
},
},
Expand All @@ -49,6 +50,7 @@ func TestConnectionInfoIterator(t *testing.T) {
SecretName: "secret-name",
AllocationEndpoints: []string{"allocation-endpoint"},
Namespace: "ns1",
ServerCA: []byte("c2VydmVyQ0E="),
},
},
},
Expand Down Expand Up @@ -233,6 +235,7 @@ func TestConnectionInfoIterator(t *testing.T) {
SecretName: "secret-name",
AllocationEndpoints: []string{"alloc1", "alloc2"},
Namespace: "ns1",
ServerCA: []byte("c2VydmVyQ0E="),
},
},
},
Expand All @@ -243,6 +246,7 @@ func TestConnectionInfoIterator(t *testing.T) {
SecretName: "secret-name",
AllocationEndpoints: []string{"alloc1", "alloc2"},
Namespace: "ns1",
ServerCA: []byte("c2VydmVyQ0E="),
},
},
},
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/multicluster/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 12 additions & 8 deletions pkg/gameserverallocations/allocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ var (
const (
secretClientCertName = "tls.crt"
secretClientKeyName = "tls.key"
secretCaCertName = "ca.crt"
secretCACertName = "ca.crt"
allocatorPort = "443"

// Instead of selecting the top one, controller selects a random one
Expand Down Expand Up @@ -324,7 +324,7 @@ func (c *Allocator) allocateFromRemoteCluster(gsa *allocationv1.GameServerAlloca
var allocationResponse *pb.AllocationResponse

// TODO: cache the client
dialOpts, err := c.createRemoteClusterDialOption(namespace, connectionInfo.SecretName)
dialOpts, err := c.createRemoteClusterDialOption(namespace, connectionInfo)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -363,13 +363,13 @@ func (c *Allocator) allocateFromRemoteCluster(gsa *allocationv1.GameServerAlloca
}

// createRemoteClusterDialOption creates a grpc client dial option with proper certs to make a remote call.
func (c *Allocator) createRemoteClusterDialOption(namespace, secretName string) (grpc.DialOption, error) {
clientCert, clientKey, caCert, err := c.getClientCertificates(namespace, secretName)
func (c *Allocator) createRemoteClusterDialOption(namespace string, connectionInfo *multiclusterv1.ClusterConnectionInfo) (grpc.DialOption, error) {
clientCert, clientKey, caCert, err := c.getClientCertificates(namespace, connectionInfo.SecretName)
if err != nil {
return nil, err
}
if clientCert == nil || clientKey == nil {
return nil, fmt.Errorf("missing client certificate key pair in secret %s", secretName)
return nil, fmt.Errorf("missing client certificate key pair in secret %s", connectionInfo.SecretName)
}

// Load client cert
Expand All @@ -379,13 +379,17 @@ func (c *Allocator) createRemoteClusterDialOption(namespace, secretName string)
}

tlsConfig := &tls.Config{Certificates: []tls.Certificate{cert}}
if len(caCert) != 0 {
if len(connectionInfo.ServerCA) != 0 || len(caCert) != 0 {
// Load CA cert, if provided and trust the server certificate.
// This is required for self-signed certs.
tlsConfig.RootCAs = x509.NewCertPool()
if !tlsConfig.RootCAs.AppendCertsFromPEM(caCert) {
if len(connectionInfo.ServerCA) != 0 && !tlsConfig.RootCAs.AppendCertsFromPEM(connectionInfo.ServerCA) {
return nil, errors.New("only PEM format is accepted for server CA")
}
// Add client CA cert, added for backward compatibility
if len(caCert) != 0 {
_ = tlsConfig.RootCAs.AppendCertsFromPEM(caCert)
}
}

return grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)), nil
Expand All @@ -404,7 +408,7 @@ func (c *Allocator) getClientCertificates(namespace, secretName string) (clientC
// Create http client using cert
clientCert = secret.Data[secretClientCertName]
clientKey = secret.Data[secretClientKeyName]
caCert = secret.Data[secretCaCertName]
caCert = secret.Data[secretCACertName]
return clientCert, clientKey, caCert, nil
}

Expand Down
48 changes: 42 additions & 6 deletions pkg/gameserverallocations/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,7 @@ func TestMultiClusterAllocationFromLocal(t *testing.T) {
ClusterName: "multicluster",
SecretName: "localhostsecret",
Namespace: defaultNs,
ServerCA: []byte("not-used"),
},
},
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -927,6 +928,7 @@ func TestMultiClusterAllocationFromRemote(t *testing.T) {
ClusterName: clusterName,
SecretName: secretName,
Namespace: targetedNamespace,
ServerCA: clientCert,
},
},
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -939,7 +941,7 @@ func TestMultiClusterAllocationFromRemote(t *testing.T) {

m.KubeClient.AddReactor("list", "secrets",
func(action k8stesting.Action) (bool, k8sruntime.Object, error) {
return true, getTestSecret(secretName, clientCert), nil
return true, getTestSecret(secretName, nil), nil
})

stop, cancel := agtesting.StartInformers(m, c.allocator.allocationPolicySynced, c.allocator.secretSynced, c.allocator.readyGameServerCache.gameServerSynced)
Expand Down Expand Up @@ -1013,6 +1015,7 @@ func TestMultiClusterAllocationFromRemote(t *testing.T) {
AllocationEndpoints: []string{endpoint},
ClusterName: clusterName,
SecretName: secretName,
ServerCA: clientCert,
},
},
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -1028,6 +1031,7 @@ func TestMultiClusterAllocationFromRemote(t *testing.T) {
AllocationEndpoints: []string{endpoint},
ClusterName: "remotecluster2",
SecretName: secretName,
ServerCA: clientCert,
},
},
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -1108,6 +1112,7 @@ func TestMultiClusterAllocationFromRemote(t *testing.T) {
AllocationEndpoints: []string{unhealthyEndpoint, healthyEndpoint},
ClusterName: clusterName,
SecretName: secretName,
ServerCA: clientCert,
},
},
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -1158,7 +1163,11 @@ func TestCreateRestClientError(t *testing.T) {
t.Parallel()
t.Run("Missing secret", func(t *testing.T) {
c, _ := newFakeController()
_, err := c.allocator.createRemoteClusterDialOption(defaultNs, "secret-name")

connectionInfo := &multiclusterv1.ClusterConnectionInfo{
SecretName: "secret-name",
}
_, err := c.allocator.createRemoteClusterDialOption(defaultNs, connectionInfo)
assert.Error(t, err)
assert.Contains(t, err.Error(), "secret-name")
})
Expand All @@ -1182,7 +1191,10 @@ func TestCreateRestClientError(t *testing.T) {
_, cancel := agtesting.StartInformers(m, c.allocator.secretSynced)
defer cancel()

_, err := c.allocator.createRemoteClusterDialOption(defaultNs, "secret-name")
connectionInfo := &multiclusterv1.ClusterConnectionInfo{
SecretName: "secret-name",
}
_, err := c.allocator.createRemoteClusterDialOption(defaultNs, connectionInfo)
assert.Error(t, err)
assert.Contains(t, err.Error(), "missing client certificate key pair in secret secret-name")
})
Expand All @@ -1207,7 +1219,10 @@ func TestCreateRestClientError(t *testing.T) {
_, cancel := agtesting.StartInformers(m, c.allocator.secretSynced)
defer cancel()

_, err := c.allocator.createRemoteClusterDialOption(defaultNs, "secret-name")
connectionInfo := &multiclusterv1.ClusterConnectionInfo{
SecretName: "secret-name",
}
_, err := c.allocator.createRemoteClusterDialOption(defaultNs, connectionInfo)
assert.Error(t, err)
assert.Contains(t, err.Error(), "failed to find any PEM data in certificate input")
})
Expand All @@ -1216,16 +1231,37 @@ func TestCreateRestClientError(t *testing.T) {

m.KubeClient.AddReactor("list", "secrets",
func(action k8stesting.Action) (bool, k8sruntime.Object, error) {
return true, getTestSecret("secret-name", []byte("XXX")), nil
return true, getTestSecret("secret-name", clientCert), nil
})

_, cancel := agtesting.StartInformers(m, c.allocator.secretSynced)
defer cancel()

_, err := c.allocator.createRemoteClusterDialOption(defaultNs, "secret-name")
connectionInfo := &multiclusterv1.ClusterConnectionInfo{
SecretName: "secret-name",
ServerCA: []byte("XXX"),
}
_, err := c.allocator.createRemoteClusterDialOption(defaultNs, connectionInfo)
assert.Error(t, err)
assert.Contains(t, err.Error(), "PEM format")
})
t.Run("Bad client CA cert", func(t *testing.T) {
c, m := newFakeController()

m.KubeClient.AddReactor("list", "secrets",
func(action k8stesting.Action) (bool, k8sruntime.Object, error) {
return true, getTestSecret("secret-name", []byte("XXX")), nil
})

_, cancel := agtesting.StartInformers(m, c.allocator.secretSynced)
defer cancel()

connectionInfo := &multiclusterv1.ClusterConnectionInfo{
SecretName: "secret-name",
}
_, err := c.allocator.createRemoteClusterDialOption(defaultNs, connectionInfo)
assert.Nil(t, err)
})
}

func executeAllocation(gsa *allocationv1.GameServerAllocation, c *Controller) (*allocationv1.GameServerAllocation, error) {
Expand Down
7 changes: 6 additions & 1 deletion site/content/en/docs/Advanced/multi-cluster-allocation.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,18 @@ spec:
clusterName: "clusterB"
namespace: cluster-B-ns
secretName: allocator-client-to-cluster-B
sercerCA: c2VydmVyQ0E=
priority: 1
weight: 100
EOF
```

To define the local cluster priority, similarly, an allocation rule should be defined, while leaving allocationEndpoints unset. If the local cluster priority is not defined, the allocation from the local cluster happens only if allocation from other clusters with the existing allocation rules is unsuccessful.

{{% feature publishVersion="1.6.0" %}}
`sercerCA` is the server TLS CA public certificate, set only if the remote server certificate is not signed by a public CA (e.g. self-signed).
{{% /feature %}}

## Establish trust

To accept allocation requests from other clusters, agones-allocator for cluster B should be configured to accept the client's certificate from cluster A and the cluster A's client should be configured to accept the server TLS certificate, if it is not signed by a public Certificate Authority (CA).
Expand All @@ -67,7 +72,7 @@ EOF

The certificates are base 64 string of the certificate file e.g. `cat ${CERT_FILE} | base64 -w 0`

`ca.crt` is the server TLS public certificate if it is self-signed. For simplicity, it is recommended to use one client secret per cluster and make `ca.crt` bundle of server certificates.
Agones recommends using [cert-manager.io](https://cert-manager.io/) solution for generating client certificates.

2.Add client CA to the list of authorized client certificates by agones-allocator in the targeted cluster.

Expand Down
11 changes: 11 additions & 0 deletions site/content/en/docs/Reference/agones_crd_api_reference.html
Original file line number Diff line number Diff line change
Expand Up @@ -5247,6 +5247,17 @@ <h3 id="ClusterConnectionInfo">ClusterConnectionInfo
<p>The cluster namespace from which to allocate gameservers</p>
</td>
</tr>
<tr>
<td>
<code>serverCa</code></br>
<em>
[]byte
</em>
</td>
<td>
<p>The PEM encoded server CA, used by the allocator client to authenticate the remote server.</p>
</td>
</tr>
</tbody>
</table>
<h3 id="ConnectionInfoIterator">ConnectionInfoIterator
Expand Down
Loading

0 comments on commit cec2f2b

Please sign in to comment.