Added allocator-client.default secret. (#1702)

* Added allocator-client.default secret. * Fixed typo. * .Release.namespace -> .Release.Namespace (and removed unnecessary whitespace). * Added testing for allocator-client.default. * Removed InsecureSkipVerify from allocator_test.go * goimports'd allocator_test.go * Added documentation, added static certificates to Helm, and tested from a non-default namespace. * Removed extra space from documentation. * Changed default value of Helm parameter to true, but false in Makefile. * Simplified client secret copying code. * Removed default certificates. Co-authored-by: Nikhil Athreya <nathreya@google.com>
googleforgames · Jul 22, 2020 · a1336de · a1336de
1 parent 91a5e62
commit a1336de
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 53 deletions.
diff --git a/build/Makefile b/build/Makefile
@@ -413,6 +413,7 @@ gen-install: $(ensure-build-image)
 		'helm template agones-manual --namespace agones-system $(mount_path)/install/helm/agones \
 		--set agones.controller.generateTLS=false \
 		--set agones.allocator.generateTLS=false \
+		--set agones.allocator.generateClientTLS=false \
 		--set agones.crds.cleanupOnDelete=false \
 		> $(mount_path)/install/yaml/install.yaml'
 

diff --git a/install/helm/agones/templates/service/allocation.yaml b/install/helm/agones/templates/service/allocation.yaml
@@ -217,6 +217,7 @@ roleRef:
 
 ---
 # Allocation CA
+{{- $selfSigned := genSelfSignedCert "" nil nil 3650 }}
 {{- $ca := genCA "allocation-ca" 3650 }}
 apiVersion: v1
 kind: Secret
@@ -229,6 +230,9 @@ metadata:
     release: "{{ .Release.Name }}"
     heritage: "{{ .Release.Service }}"
 data:
+{{- if .Values.agones.allocator.generateClientTLS }}
+  allocator-client.default.crt: {{ b64enc $selfSigned.Cert }}
+{{- end }}
 {{- if .Values.agones.allocator.generateTLS }}
   client-ca.crt: {{ b64enc $ca.Cert }}
 {{- else }}
@@ -276,3 +280,24 @@ data:
 {{- else }}
   tls-ca.crt: {{ .Files.Get "certs/allocator/server.crt" | b64enc }}
 {{- end }}
+
+---
+# Default allocation client secret
+{{- if .Values.agones.allocator.generateClientTLS }}
+{{- range .Values.gameservers.namespaces }}
+apiVersion: v1
+kind: Secret
+type: kubernetes.io/tls
+metadata:
+  name: allocator-client.default
+  namespace: {{ . }}
+  labels:
+    app: {{ template "agones.fullname" $ }}
+    chart: "{{ $.Chart.Name }}-{{ $.Chart.Version }}"
+    release: "{{ $.Release.Name }}"
+    heritage: "{{ $.Release.Service }}"
+data:
+  tls.crt: {{ b64enc $selfSigned.Cert }}
+  tls.key: {{ b64enc $selfSigned.Key }}
+{{- end }}
+{{- end }}
diff --git a/install/helm/agones/values.yaml b/install/helm/agones/values.yaml
@@ -126,6 +126,7 @@ agones:
       serviceType: LoadBalancer
       annotations: {}
     generateTLS: true
+    generateClientTLS: true
     disableMTLS: false
   image:
     registry: gcr.io/agones-images

diff --git a/install/yaml/install.yaml b/install/yaml/install.yaml
@@ -1635,6 +1635,9 @@ spec:
 # See the License for the specific language governing permissions and
 # limitations under the License.
 ---
+# Source: agones/templates/service/allocation.yaml
+# Default allocation client secret
+---
 # Source: agones/templates/extensions.yaml
 apiVersion: admissionregistration.k8s.io/v1beta1
 kind: MutatingWebhookConfiguration

diff --git a/site/content/en/docs/Installation/Install Agones/helm.md b/site/content/en/docs/Installation/Install Agones/helm.md
@@ -214,6 +214,7 @@ The following tables lists the configurable parameters of the Agones chart and t
 | `agones.allocator.replicas`                         | The number of replicas to run in the deployment                                                 | `3`                    |
 | `agones.allocator.http.port`                        | The port to expose on the service                                                               | `443`                  |
 | `agones.allocator.http.serviceType`                 | The [Service Type][service] of the HTTP Service                                                 | `LoadBalancer`         |
+| `agones.allocator.generateClientTLS`                | Set to true to generate client TLS certificates or false to provide certificates in `certs/allocator/allocator-client.default/*` | `true`                 |
 | `agones.allocator.generateTLS`                      | Set to true to generate TLS certificates or false to provide certificates in `certs/allocator/*`| `true`                 |
 | `agones.allocator.tolerations`                      | Allocator [toleration][toleration] labels for pod assignment                                    | `[]`                   |
 | `agones.allocator.affinity`                         | Allocator [affinity][affinity] settings for pod assignment                                      | `{}`                   |

diff --git a/test/e2e/allocator_test.go b/test/e2e/allocator_test.go
@@ -37,30 +37,27 @@ import (
 	"github.com/stretchr/testify/assert"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
-	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/apimachinery/pkg/util/wait"
 )
 
 const (
-	agonesSystemNamespace = "agones-system"
-	allocatorServiceName  = "agones-allocator"
-	allocatorTLSName      = "allocator-tls"
-	allocatorClientCAName = "allocator-client-ca"
-	tlsCrtTag             = "tls.crt"
-	tlsKeyTag             = "tls.key"
-	allocatorReqURLFmt    = "%s:%d"
+	agonesSystemNamespace          = "agones-system"
+	allocatorServiceName           = "agones-allocator"
+	allocatorTLSName               = "allocator-tls"
+	tlsCrtTag                      = "tls.crt"
+	tlsKeyTag                      = "tls.key"
+	allocatorReqURLFmt             = "%s:%d"
+	allocatorClientSecretName      = "allocator-client.default"
+	allocatorClientSecretNamespace = "default"
 )
 
 func TestAllocator(t *testing.T) {
 	ip, port := getAllocatorEndpoint(t)
 	requestURL := fmt.Sprintf(allocatorReqURLFmt, ip, port)
 	tlsCA := refreshAllocatorTLSCerts(t, ip)
 
-	clientSecretName := fmt.Sprintf("allocator-client-%s", uuid.NewUUID())
-	genClientSecret(t, framework.Namespace, clientSecretName)
-
 	flt, err := createFleet(framework.Namespace)
 	if !assert.Nil(t, err) {
 		return
@@ -77,7 +74,7 @@ func TestAllocator(t *testing.T) {
 	// wait for the allocation system to come online
 	err = wait.PollImmediate(2*time.Second, 5*time.Minute, func() (bool, error) {
 		// create the grpc client each time, as we may end up looking at an old cert
-		dialOpts, err := createRemoteClusterDialOption(framework.Namespace, clientSecretName, tlsCA)
+		dialOpts, err := createRemoteClusterDialOption(allocatorClientSecretNamespace, allocatorClientSecretName, tlsCA)
 		if err != nil {
 			return false, err
 		}
@@ -112,6 +109,7 @@ func TestAllocatorCrossNamespace(t *testing.T) {
 
 	// Create namespaces A and B
 	namespaceA := framework.Namespace // let's reuse an existing one
+	copyDefaultAllocatorClientSecret(t, namespaceA)
 
 	namespaceB := fmt.Sprintf("allocator-b-%s", uuid.NewUUID())
 	err := framework.CreateNamespace(namespaceB)
@@ -124,10 +122,6 @@ func TestAllocatorCrossNamespace(t *testing.T) {
 		}
 	}()
 
-	// Create client secret A, B is receiver of the request and does not need client secret
-	clientSecretNameA := fmt.Sprintf("allocator-client-%s", uuid.NewUUID())
-	genClientSecret(t, namespaceA, clientSecretNameA)
-
 	policyName := fmt.Sprintf("a-to-b-%s", uuid.NewUUID())
 	p := &multiclusterv1.GameServerAllocationPolicy{
 		ObjectMeta: metav1.ObjectMeta{
@@ -138,7 +132,7 @@ func TestAllocatorCrossNamespace(t *testing.T) {
 			Priority: 1,
 			Weight:   1,
 			ConnectionInfo: multiclusterv1.ClusterConnectionInfo{
-				SecretName:          clientSecretNameA,
+				SecretName:          allocatorClientSecretName,
 				Namespace:           namespaceB,
 				AllocationEndpoints: []string{ip},
 				ServerCA:            tlsCA,
@@ -163,7 +157,7 @@ func TestAllocatorCrossNamespace(t *testing.T) {
 	// wait for the allocation system to come online
 	err = wait.PollImmediate(2*time.Second, 5*time.Minute, func() (bool, error) {
 		// create the grpc client each time, as we may end up looking at an old cert
-		dialOpts, err := createRemoteClusterDialOption(namespaceA, clientSecretNameA, tlsCA)
+		dialOpts, err := createRemoteClusterDialOption(namespaceA, allocatorClientSecretName, tlsCA)
 		if err != nil {
 			return false, err
 		}
@@ -188,6 +182,20 @@ func TestAllocatorCrossNamespace(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func copyDefaultAllocatorClientSecret(t *testing.T, toNamespace string) {
+	kubeCore := framework.KubeClient.CoreV1()
+	clientSecret, err := kubeCore.Secrets(allocatorClientSecretNamespace).Get(allocatorClientSecretName, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("Could not retrieve default allocator client secret %s/%s: %v", allocatorClientSecretNamespace, allocatorClientSecretName, err)
+	}
+	clientSecret.ObjectMeta.Namespace = toNamespace
+	clientSecret.ResourceVersion = ""
+	_, err = kubeCore.Secrets(toNamespace).Create(clientSecret)
+	if err != nil {
+		t.Fatalf("Could not copy default allocator client %s/%s secret to namespace %s: %v", allocatorClientSecretNamespace, allocatorClientSecretName, toNamespace, err)
+	}
+}
+
 func createAllocationPolicy(t *testing.T, p *multiclusterv1.GameServerAllocationPolicy) {
 	t.Helper()
 
@@ -259,41 +267,6 @@ func createFleet(namespace string) (*agonesv1.Fleet, error) {
 	return fleets.Create(fleet)
 }
 
-func genClientSecret(t *testing.T, namespace, secretName string) {
-	t.Helper()
-
-	pub, priv := generateTLSCertPair(t, "")
-
-	kubeCore := framework.KubeClient.CoreV1()
-	s := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      secretName,
-			Namespace: namespace,
-		},
-		Data: map[string][]byte{
-			tlsCrtTag: pub,
-			tlsKeyTag: priv,
-		},
-	}
-	if _, err := kubeCore.Secrets(namespace).Create(s); err != nil {
-		t.Fatalf("Creating secret %s/%s failed: %s", namespace, secretName, err)
-	}
-	t.Logf("Client secret is created: %v", s)
-
-	// Add client CA to authorized client CAs
-	s, err := kubeCore.Secrets(agonesSystemNamespace).Get(allocatorClientCAName, metav1.GetOptions{})
-	if err != nil {
-		t.Fatalf("getting secret %s/%s failed: %s", agonesSystemNamespace, allocatorClientCAName, err)
-	}
-
-	s.Data["client-ca.crt"] = pub
-	clientCASecret, err := kubeCore.Secrets(agonesSystemNamespace).Update(s)
-	if err != nil {
-		t.Fatalf("updating secrets failed: %s", err)
-	}
-	t.Logf("Secret is updated: %v", clientCASecret)
-}
-
 func refreshAllocatorTLSCerts(t *testing.T, host string) []byte {
 	t.Helper()