From 50b03f100885ce09fe57e0036935d4c9673754e9 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Tue, 8 Mar 2022 20:15:43 +0000 Subject: [PATCH 1/6] support multiple Gateways/contour instances per ns Signed-off-by: Steve Kriss --- .../objects/configmap/configmap.go | 2 +- .../objects/configmap/configmap_test.go | 4 +- .../objects/daemonset/daemonset.go | 18 ++-- .../objects/deployment/deployment.go | 24 ++--- internal/provisioner/objects/job/job.go | 13 ++- internal/provisioner/objects/rbac.go | 101 +++++++++--------- .../provisioner/objects/service/service.go | 12 +-- internal/provisioner/validation/validation.go | 3 - 8 files changed, 85 insertions(+), 92 deletions(-) diff --git a/internal/provisioner/objects/configmap/configmap.go b/internal/provisioner/objects/configmap/configmap.go index d4915c5a965..f4b8484f546 100644 --- a/internal/provisioner/objects/configmap/configmap.go +++ b/internal/provisioner/objects/configmap/configmap.go @@ -183,7 +183,7 @@ type contourConfig struct { func configForContour(contour *model.Contour) *configMapParams { return &configMapParams{ Namespace: contour.Namespace, - Name: ContourConfigMapName, + Name: fmt.Sprintf("%s-%s", ContourConfigMapName, contour.Name), Labels: model.OwnerLabels(contour), Contour: contourConfig{ GatewayNamespace: contour.Namespace, diff --git a/internal/provisioner/objects/configmap/configmap_test.go b/internal/provisioner/objects/configmap/configmap_test.go index 7eeb59877c6..8368d9053fa 100644 --- a/internal/provisioner/objects/configmap/configmap_test.go +++ b/internal/provisioner/objects/configmap/configmap_test.go @@ -142,7 +142,7 @@ accesslog-format: envoy } cm, err := desired(configForContour(c)) require.NoError(t, err) - require.Equal(t, "contour", cm.Name) + require.Equal(t, "contour-test", cm.Name) require.Equal(t, "test-ns", cm.Namespace) require.Contains(t, cm.Data, "contour.yaml") assert.Equal(t, expected, cm.Data["contour.yaml"]) @@ -392,7 +392,7 @@ accesslog-format: envoy } cm, err := desired(configForContour(c)) require.NoError(t, err) - require.Equal(t, "contour", cm.Name) + require.Equal(t, "contour-test", cm.Name) require.Equal(t, "test-ns", cm.Namespace) require.Contains(t, cm.Data, "contour.yaml") assert.Equal(t, expected, cm.Data["contour.yaml"]) diff --git a/internal/provisioner/objects/daemonset/daemonset.go b/internal/provisioner/objects/daemonset/daemonset.go index e1daea7d878..046b82cdb10 100644 --- a/internal/provisioner/objects/daemonset/daemonset.go +++ b/internal/provisioner/objects/daemonset/daemonset.go @@ -268,7 +268,7 @@ func DesiredDaemonSet(contour *model.Contour, contourImage, envoyImage string) * Args: []string{ "bootstrap", filepath.Join("/", envoyCfgVolMntDir, envoyCfgFileName), - "--xds-address=contour", + "--xds-address=contour-" + contour.Name, fmt.Sprintf("--xds-port=%d", objcfg.XDSPort), fmt.Sprintf("--xds-resource-version=%s", xdsResourceVersion), fmt.Sprintf("--resources-dir=%s", filepath.Join("/", envoyCfgVolMntDir, "resources")), @@ -307,13 +307,13 @@ func DesiredDaemonSet(contour *model.Contour, contourImage, envoyImage string) * ds := &appsv1.DaemonSet{ ObjectMeta: metav1.ObjectMeta{ Namespace: contour.Namespace, - Name: envoyDaemonSetName, + Name: fmt.Sprintf("%s-%s", envoyDaemonSetName, contour.Name), Labels: labels, }, Spec: appsv1.DaemonSetSpec{ RevisionHistoryLimit: pointer.Int32Ptr(int32(10)), // Ensure the deamonset adopts only its own pods. - Selector: EnvoyDaemonSetPodSelector(), + Selector: EnvoyDaemonSetPodSelector(contour.Name), UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ Type: appsv1.RollingUpdateDaemonSetStrategyType, RollingUpdate: &appsv1.RollingUpdateDaemonSet{ @@ -329,7 +329,7 @@ func DesiredDaemonSet(contour *model.Contour, contourImage, envoyImage string) * "prometheus.io/port": "8002", "prometheus.io/path": "/stats/prometheus", }, - Labels: EnvoyDaemonSetPodSelector().MatchLabels, + Labels: EnvoyDaemonSetPodSelector(contour.Name).MatchLabels, }, Spec: corev1.PodSpec{ Containers: containers, @@ -340,7 +340,7 @@ func DesiredDaemonSet(contour *model.Contour, contourImage, envoyImage string) * VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ DefaultMode: pointer.Int32Ptr(int32(420)), - SecretName: envoyCertsSecretName, + SecretName: fmt.Sprintf("%s-%s", contour.Name, envoyCertsSecretName), }, }, }, @@ -357,7 +357,7 @@ func DesiredDaemonSet(contour *model.Contour, contourImage, envoyImage string) * }, }, }, - ServiceAccountName: objutil.EnvoyRbacName, + ServiceAccountName: fmt.Sprintf("%s-%s", objutil.EnvoyRbacName, contour.Name), AutomountServiceAccountToken: pointer.BoolPtr(false), TerminationGracePeriodSeconds: pointer.Int64Ptr(int64(300)), SecurityContext: objutil.NewUnprivilegedPodSecurity(), @@ -385,7 +385,7 @@ func CurrentDaemonSet(ctx context.Context, cli client.Client, contour *model.Con ds := &appsv1.DaemonSet{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: envoyDaemonSetName, + Name: fmt.Sprintf("%s-%s", envoyDaemonSetName, contour.Name), } if err := cli.Get(ctx, key, ds); err != nil { return nil, err @@ -418,10 +418,10 @@ func updateDaemonSetIfNeeded(ctx context.Context, cli client.Client, contour *mo // EnvoyDaemonSetPodSelector returns a label selector using "app: envoy" as the // key/value pair. -func EnvoyDaemonSetPodSelector() *metav1.LabelSelector { +func EnvoyDaemonSetPodSelector(contourName string) *metav1.LabelSelector { return &metav1.LabelSelector{ MatchLabels: map[string]string{ - "app": "envoy", + "app": "envoy-" + contourName, }, } } diff --git a/internal/provisioner/objects/deployment/deployment.go b/internal/provisioner/objects/deployment/deployment.go index 382f9915f13..aee333d5112 100644 --- a/internal/provisioner/objects/deployment/deployment.go +++ b/internal/provisioner/objects/deployment/deployment.go @@ -122,6 +122,8 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment fmt.Sprintf("--contour-cert-file=%s", filepath.Join("/", contourCertsVolMntDir, "tls.crt")), fmt.Sprintf("--contour-key-file=%s", filepath.Join("/", contourCertsVolMntDir, "tls.key")), fmt.Sprintf("--config-path=%s", filepath.Join("/", contourCfgVolMntDir, contourCfgFileName)), + fmt.Sprintf("--leader-election-resource-name=%s", "leader-elect-"+contour.Name), + fmt.Sprintf("--envoy-service-name=%s", "envoy-"+contour.Name), } // Pass the insecure/secure flags to Contour if using non-default ports. for _, port := range contour.Spec.NetworkPublishing.Envoy.ContainerPorts { @@ -223,7 +225,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment deploy := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Namespace: contour.Namespace, - Name: contourDeploymentName, + Name: fmt.Sprintf("%s-%s", contourDeploymentName, contour.Name), Labels: makeDeploymentLabels(contour), }, Spec: appsv1.DeploymentSpec{ @@ -231,7 +233,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment Replicas: &contour.Spec.Replicas, RevisionHistoryLimit: pointer.Int32Ptr(int32(10)), // Ensure the deployment adopts only its own pods. - Selector: ContourDeploymentPodSelector(), + Selector: ContourDeploymentPodSelector(contour.Name), Strategy: appsv1.DeploymentStrategy{ Type: appsv1.RollingUpdateDeploymentStrategyType, RollingUpdate: &appsv1.RollingUpdateDeployment{ @@ -247,7 +249,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment "prometheus.io/scrape": "true", "prometheus.io/port": fmt.Sprintf("%d", metricsPort), }, - Labels: ContourDeploymentPodSelector().MatchLabels, + Labels: ContourDeploymentPodSelector(contour.Name).MatchLabels, }, Spec: corev1.PodSpec{ // TODO [danehans]: Readdress anti-affinity when https://github.com/projectcontour/contour/issues/2997 @@ -260,7 +262,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment PodAffinityTerm: corev1.PodAffinityTerm{ TopologyKey: "kubernetes.io/hostname", LabelSelector: &metav1.LabelSelector{ - MatchLabels: ContourDeploymentPodSelector().MatchLabels, + MatchLabels: ContourDeploymentPodSelector(contour.Name).MatchLabels, }, }, }, @@ -274,7 +276,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ DefaultMode: pointer.Int32Ptr(int32(420)), - SecretName: contourCertsSecretName, + SecretName: fmt.Sprintf("%s-%s", contour.Name, contourCertsSecretName), }, }, }, @@ -283,9 +285,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ - // [TODO] danehans: Update to contour.Name when - // projectcontour/contour/issues/2122 is fixed. - Name: objcm.ContourConfigMapName, + Name: fmt.Sprintf("%s-%s", objcm.ContourConfigMapName, contour.Name), }, Items: []corev1.KeyToPath{ { @@ -299,7 +299,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment }, }, DNSPolicy: corev1.DNSClusterFirst, - ServiceAccountName: objutil.ContourRbacName, + ServiceAccountName: fmt.Sprintf("%s-%s", objutil.ContourRbacName, contour.Name), RestartPolicy: corev1.RestartPolicyAlways, SchedulerName: "default-scheduler", SecurityContext: objutil.NewUnprivilegedPodSecurity(), @@ -325,7 +325,7 @@ func CurrentDeployment(ctx context.Context, cli client.Client, contour *model.Co deploy := &appsv1.Deployment{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: contourDeploymentName, + Name: fmt.Sprintf("%s-%s", contourDeploymentName, contour.Name), } if err := cli.Get(ctx, key, deploy); err != nil { return nil, err @@ -374,10 +374,10 @@ func makeDeploymentLabels(contour *model.Contour) map[string]string { // ContourDeploymentPodSelector returns a label selector using "app: contour" as the // key/value pair. -func ContourDeploymentPodSelector() *metav1.LabelSelector { +func ContourDeploymentPodSelector(contourName string) *metav1.LabelSelector { return &metav1.LabelSelector{ MatchLabels: map[string]string{ - "app": "contour", + "app": "contour-" + contourName, }, } } diff --git a/internal/provisioner/objects/job/job.go b/internal/provisioner/objects/job/job.go index 8bb3df700c2..4bd8a6afa56 100644 --- a/internal/provisioner/objects/job/job.go +++ b/internal/provisioner/objects/job/job.go @@ -38,10 +38,8 @@ const ( jobNsEnvVar = "CONTOUR_NAMESPACE" ) -func certgenJobName(contourImage string) string { - // [TODO] danehans: Remove and use contour.Name + "-certgen" when - // https://github.com/projectcontour/contour/issues/2122 is fixed. - return "contour-certgen-" + objutil.TagFromImage(contourImage) +func certgenJobName(contourImage, contourName string) string { + return fmt.Sprintf("%s-%s-%s", "contour-certgen", objutil.TagFromImage(contourImage), contourName) } // EnsureJob ensures that a Job exists for the given contour. @@ -89,7 +87,7 @@ func currentJob(ctx context.Context, cli client.Client, contour *model.Contour, current := &batchv1.Job{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: certgenJobName(image), + Name: certgenJobName(image, contour.Name), } err := cli.Get(ctx, key, current) if err != nil { @@ -121,6 +119,7 @@ func DesiredJob(contour *model.Contour, image string) *batchv1.Job { "--overwrite", "--secrets-format=compact", fmt.Sprintf("--namespace=$(%s)", jobNsEnvVar), + fmt.Sprintf("--secrets-name-prefix=%s", contour.Name), }, Env: []corev1.EnvVar{env}, TerminationMessagePath: "/dev/termination-log", @@ -128,7 +127,7 @@ func DesiredJob(contour *model.Contour, image string) *batchv1.Job { } spec := corev1.PodSpec{ Containers: []corev1.Container{container}, - ServiceAccountName: objutil.CertGenRbacName, + ServiceAccountName: fmt.Sprintf("%s-%s", objutil.CertGenRbacName, contour.Name), SecurityContext: objutil.NewUnprivilegedPodSecurity(), RestartPolicy: corev1.RestartPolicyNever, DNSPolicy: corev1.DNSClusterFirst, @@ -151,7 +150,7 @@ func DesiredJob(contour *model.Contour, image string) *batchv1.Job { job := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ - Name: certgenJobName(image), + Name: certgenJobName(image, contour.Name), Namespace: contour.Namespace, Labels: labels, }, diff --git a/internal/provisioner/objects/rbac.go b/internal/provisioner/objects/rbac.go index 3079bcd9f51..b01ce015ae0 100644 --- a/internal/provisioner/objects/rbac.go +++ b/internal/provisioner/objects/rbac.go @@ -50,34 +50,37 @@ func EnsureRBAC(ctx context.Context, cli client.Client, contour *model.Contour) ns := contour.Namespace names := []string{ContourRbacName, EnvoyRbacName, CertGenRbacName} for _, name := range names { - _, err := objsa.EnsureServiceAccount(ctx, cli, name, contour) + saName := fmt.Sprintf("%s-%s", name, contour.Name) + + _, err := objsa.EnsureServiceAccount(ctx, cli, saName, contour) if err != nil { - return fmt.Errorf("failed to ensure service account %s/%s: %w", ns, name, err) + return fmt.Errorf("failed to ensure service account %s/%s: %w", ns, saName, err) } } // ClusterRole and ClusterRoleBinding resources are namespace-named to allow ownership // from individual instances of Contour. - nsName := fmt.Sprintf("%s-%s", ContourRbacName, ns) + nsName := fmt.Sprintf("%s-%s-%s", ContourRbacName, ns, contour.Name) cr, err := objcr.EnsureClusterRole(ctx, cli, nsName, contour) if err != nil { - return fmt.Errorf("failed to ensure cluster role %s: %w", ContourRbacName, err) + return fmt.Errorf("failed to ensure cluster role %s: %w", nsName, err) } - if err := objcrb.EnsureClusterRoleBinding(ctx, cli, nsName, cr.Name, ContourRbacName, contour); err != nil { - return fmt.Errorf("failed to ensure cluster role binding %s: %w", ContourRbacName, err) + if err := objcrb.EnsureClusterRoleBinding(ctx, cli, nsName, cr.Name, fmt.Sprintf("%s-%s", ContourRbacName, contour.Name), contour); err != nil { + return fmt.Errorf("failed to ensure cluster role binding %s: %w", fmt.Sprintf("%s-%s", ContourRbacName, contour.Name), err) } - certRole, err := objrole.EnsureCertgenRole(ctx, cli, CertGenRbacName, contour) + certRole, err := objrole.EnsureCertgenRole(ctx, cli, fmt.Sprintf("%s-%s", CertGenRbacName, contour.Name), contour) if err != nil { - return fmt.Errorf("failed to ensure certgen role %s/%s: %w", ns, CertGenRbacName, err) + return fmt.Errorf("failed to ensure certgen role %s/%s: %w", ns, fmt.Sprintf("%s-%s", CertGenRbacName, contour.Name), err) } - if err := objrb.EnsureRoleBinding(ctx, cli, ContourRbacName, CertGenRbacName, certRole.Name, contour); err != nil { + // this one is named "contour-" despite being for certgen. + if err := objrb.EnsureRoleBinding(ctx, cli, fmt.Sprintf("%s-%s", ContourRbacName, contour.Name), fmt.Sprintf("%s-%s", CertGenRbacName, contour.Name), certRole.Name, contour); err != nil { return fmt.Errorf("failed to ensure certgen role binding %s/%s: %w", ns, ContourRbacName, err) } - controllerRole, err := objrole.EnsureControllerRole(ctx, cli, ContourRbacName, contour) + controllerRole, err := objrole.EnsureControllerRole(ctx, cli, fmt.Sprintf("%s-%s", ContourRbacName, contour.Name), contour) if err != nil { - return fmt.Errorf("failed to ensure controller role %s/%s: %w", ns, ContourRbacName, err) + return fmt.Errorf("failed to ensure controller role %s/%s: %w", ns, fmt.Sprintf("%s-%s", ContourRbacName, contour.Name), err) } - if err := objrb.EnsureRoleBinding(ctx, cli, ContourRoleBindingName, ContourRbacName, controllerRole.Name, contour); err != nil { - return fmt.Errorf("failed to ensure controller role binding %s/%s: %w", ns, ContourRbacName, err) + if err := objrb.EnsureRoleBinding(ctx, cli, fmt.Sprintf("%s-%s", ContourRoleBindingName, contour.Name), fmt.Sprintf("%s-%s", ContourRbacName, contour.Name), controllerRole.Name, contour); err != nil { + return fmt.Errorf("failed to ensure controller role binding %s/%s: %w", ns, fmt.Sprintf("%s-%s", ContourRoleBindingName, contour.Name), err) } return nil } @@ -89,48 +92,42 @@ func EnsureRBACDeleted(ctx context.Context, cli client.Client, contour *model.Co ns := contour.Namespace objectsToDelete := []client.Object{} - // TODO(sk) right now we can't support running more than one Contour instance - // per namespace, so just assume there's only one. - otherContoursExistInSpecNs := false - - if !otherContoursExistInSpecNs { - controllerRoleBind, err := objrb.CurrentRoleBinding(ctx, cli, ns, ContourRoleBindingName) - if err != nil && !errors.IsNotFound(err) { - return err - } - if controllerRoleBind != nil { - objectsToDelete = append(objectsToDelete, controllerRoleBind) - } - controllerRole, err := objrole.CurrentRole(ctx, cli, ns, ContourRbacName) - if err != nil && !errors.IsNotFound(err) { - return err - } - if controllerRole != nil { - objectsToDelete = append(objectsToDelete, controllerRole) - } - certRoleBind, err := objrb.CurrentRoleBinding(ctx, cli, ns, ContourRbacName) - if err != nil && !errors.IsNotFound(err) { - return err - } - if certRoleBind != nil { - objectsToDelete = append(objectsToDelete, certRoleBind) - } - certRole, err := objrole.CurrentRole(ctx, cli, ns, CertGenRbacName) + controllerRoleBind, err := objrb.CurrentRoleBinding(ctx, cli, ns, ContourRoleBindingName) + if err != nil && !errors.IsNotFound(err) { + return err + } + if controllerRoleBind != nil { + objectsToDelete = append(objectsToDelete, controllerRoleBind) + } + controllerRole, err := objrole.CurrentRole(ctx, cli, ns, ContourRbacName) + if err != nil && !errors.IsNotFound(err) { + return err + } + if controllerRole != nil { + objectsToDelete = append(objectsToDelete, controllerRole) + } + certRoleBind, err := objrb.CurrentRoleBinding(ctx, cli, ns, ContourRbacName) + if err != nil && !errors.IsNotFound(err) { + return err + } + if certRoleBind != nil { + objectsToDelete = append(objectsToDelete, certRoleBind) + } + certRole, err := objrole.CurrentRole(ctx, cli, ns, CertGenRbacName) + if err != nil && !errors.IsNotFound(err) { + return err + } + if certRole != nil { + objectsToDelete = append(objectsToDelete, certRole) + } + names := []string{ContourRbacName, EnvoyRbacName, CertGenRbacName} + for _, name := range names { + svcAct, err := objsa.CurrentServiceAccount(ctx, cli, ns, name) if err != nil && !errors.IsNotFound(err) { return err } - if certRole != nil { - objectsToDelete = append(objectsToDelete, certRole) - } - names := []string{ContourRbacName, EnvoyRbacName, CertGenRbacName} - for _, name := range names { - svcAct, err := objsa.CurrentServiceAccount(ctx, cli, ns, name) - if err != nil && !errors.IsNotFound(err) { - return err - } - if svcAct != nil { - objectsToDelete = append(objectsToDelete, svcAct) - } + if svcAct != nil { + objectsToDelete = append(objectsToDelete, svcAct) } } diff --git a/internal/provisioner/objects/service/service.go b/internal/provisioner/objects/service/service.go index 17711f8a4ca..c4fb5af041b 100644 --- a/internal/provisioner/objects/service/service.go +++ b/internal/provisioner/objects/service/service.go @@ -196,7 +196,7 @@ func DesiredContourService(contour *model.Contour) *corev1.Service { svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Namespace: contour.Namespace, - Name: contourSvcName, + Name: fmt.Sprintf("%s-%s", contourSvcName, contour.Name), Labels: model.OwnerLabels(contour), }, Spec: corev1.ServiceSpec{ @@ -208,7 +208,7 @@ func DesiredContourService(contour *model.Contour) *corev1.Service { TargetPort: intstr.IntOrString{IntVal: xdsPort}, }, }, - Selector: objdeploy.ContourDeploymentPodSelector().MatchLabels, + Selector: objdeploy.ContourDeploymentPodSelector(contour.Name).MatchLabels, Type: corev1.ServiceTypeClusterIP, SessionAffinity: corev1.ServiceAffinityNone, }, @@ -264,13 +264,13 @@ func DesiredEnvoyService(contour *model.Contour) *corev1.Service { svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Namespace: contour.Namespace, - Name: envoySvcName, + Name: fmt.Sprintf("%s-%s", envoySvcName, contour.Name), Annotations: map[string]string{}, Labels: model.OwnerLabels(contour), }, Spec: corev1.ServiceSpec{ Ports: ports, - Selector: objds.EnvoyDaemonSetPodSelector().MatchLabels, + Selector: objds.EnvoyDaemonSetPodSelector(contour.Name).MatchLabels, SessionAffinity: corev1.ServiceAffinityNone, }, } @@ -356,7 +356,7 @@ func currentContourService(ctx context.Context, cli client.Client, contour *mode current := &corev1.Service{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: contourSvcName, + Name: fmt.Sprintf("%s-%s", contourSvcName, contour.Name), } err := cli.Get(ctx, key, current) if err != nil { @@ -370,7 +370,7 @@ func currentEnvoyService(ctx context.Context, cli client.Client, contour *model. current := &corev1.Service{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: envoySvcName, + Name: fmt.Sprintf("%s-%s", envoySvcName, contour.Name), } err := cli.Get(ctx, key, current) if err != nil { diff --git a/internal/provisioner/validation/validation.go b/internal/provisioner/validation/validation.go index 2b7a687db9c..5829aa50b8f 100644 --- a/internal/provisioner/validation/validation.go +++ b/internal/provisioner/validation/validation.go @@ -25,9 +25,6 @@ import ( // Contour returns true if contour is valid. func Contour(ctx context.Context, cli client.Client, contour *model.Contour) error { - // TODO(sk) this used to check for existence of another Contour in the namespace, - // we likely want to do the same for Gateways somehow. - if err := ContainerPorts(contour); err != nil { return err } From 3908fdb1817bb681fc028f07ab9b12947ee2f724 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Mon, 28 Mar 2022 22:09:48 +0000 Subject: [PATCH 2/6] set envoy-service-name correctly Signed-off-by: Steve Kriss --- internal/provisioner/objects/configmap/configmap.go | 7 +++++++ internal/provisioner/objects/configmap/configmap_test.go | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/internal/provisioner/objects/configmap/configmap.go b/internal/provisioner/objects/configmap/configmap.go index f4b8484f546..ec9520a297a 100644 --- a/internal/provisioner/objects/configmap/configmap.go +++ b/internal/provisioner/objects/configmap/configmap.go @@ -150,6 +150,8 @@ accesslog-format: envoy # Configure the number of additional ingress proxy hops from the # right side of the x-forwarded-for HTTP header to trust. # num-trusted-hops: 0 +# Name of the envoy service to inspect for Ingress status details. +envoy-service-name: {{ .EnvoyServiceName }} `)) // configMapParams contains everything needed to manage a Contour ConfigMap. @@ -177,6 +179,10 @@ type contourConfig struct { // EnableExternalNameService sets whether ExternalName Services are // allowed. EnableExternalNameService bool + + // EnvoyServiceName is the name of the envoy service to inspect for Ingress + // status details. + EnvoyServiceName string } // configForContour returns a configMapParams with default fields set for contour. @@ -189,6 +195,7 @@ func configForContour(contour *model.Contour) *configMapParams { GatewayNamespace: contour.Namespace, GatewayName: contour.Name, EnableExternalNameService: pointer.BoolDeref(contour.Spec.EnableExternalNameService, false), + EnvoyServiceName: fmt.Sprintf("envoy-%s", contour.Name), }, } } diff --git a/internal/provisioner/objects/configmap/configmap_test.go b/internal/provisioner/objects/configmap/configmap_test.go index 8368d9053fa..d3535f654b8 100644 --- a/internal/provisioner/objects/configmap/configmap_test.go +++ b/internal/provisioner/objects/configmap/configmap_test.go @@ -132,6 +132,8 @@ accesslog-format: envoy # Configure the number of additional ingress proxy hops from the # right side of the x-forwarded-for HTTP header to trust. # num-trusted-hops: 0 +# Name of the envoy service to inspect for Ingress status details. +envoy-service-name: envoy-test ` c := &model.Contour{ @@ -257,6 +259,8 @@ accesslog-format: envoy # Configure the number of additional ingress proxy hops from the # right side of the x-forwarded-for HTTP header to trust. # num-trusted-hops: 0 +# Name of the envoy service to inspect for Ingress status details. +envoy-service-name: envoy-test ` c := &model.Contour{ ObjectMeta: v1.ObjectMeta{ @@ -382,6 +386,8 @@ accesslog-format: envoy # Configure the number of additional ingress proxy hops from the # right side of the x-forwarded-for HTTP header to trust. # num-trusted-hops: 0 +# Name of the envoy service to inspect for Ingress status details. +envoy-service-name: envoy-test ` c := &model.Contour{ From 8a7a8abf7c7925cf8a2180a0163d03117e62f585 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Thu, 10 Mar 2022 16:10:55 +0000 Subject: [PATCH 3/6] clean up resource name generation Signed-off-by: Steve Kriss --- .../objects/configmap/configmap.go | 12 +- .../objects/daemonset/daemonset.go | 17 +- .../objects/deployment/deployment.go | 19 +- internal/provisioner/objects/job/job.go | 2 +- internal/provisioner/objects/rbac.go | 280 +++++++++++------- .../provisioner/objects/service/service.go | 30 +- 6 files changed, 221 insertions(+), 139 deletions(-) diff --git a/internal/provisioner/objects/configmap/configmap.go b/internal/provisioner/objects/configmap/configmap.go index ec9520a297a..755d56dfd23 100644 --- a/internal/provisioner/objects/configmap/configmap.go +++ b/internal/provisioner/objects/configmap/configmap.go @@ -31,12 +31,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -const ( - // ContourConfigMapName is the name of Contour's ConfigMap resource. - // [TODO] danehans: Remove and use contour.Name when - // https://github.com/projectcontour/contour/issues/2122 is fixed. - ContourConfigMapName = "contour" -) +// ContourConfigMapName returns the name of Contour's ConfigMap resource. +func ContourConfigMapName(contour *model.Contour) string { + return fmt.Sprintf("%s-%s", "contour", contour.Name) +} var contourConfigMapTemplate = template.Must(template.New("contour.yaml").Parse(`# # server: @@ -189,7 +187,7 @@ type contourConfig struct { func configForContour(contour *model.Contour) *configMapParams { return &configMapParams{ Namespace: contour.Namespace, - Name: fmt.Sprintf("%s-%s", ContourConfigMapName, contour.Name), + Name: ContourConfigMapName(contour), Labels: model.OwnerLabels(contour), Contour: contourConfig{ GatewayNamespace: contour.Namespace, diff --git a/internal/provisioner/objects/daemonset/daemonset.go b/internal/provisioner/objects/daemonset/daemonset.go index 046b82cdb10..125e4fc6482 100644 --- a/internal/provisioner/objects/daemonset/daemonset.go +++ b/internal/provisioner/objects/daemonset/daemonset.go @@ -36,10 +36,8 @@ import ( ) const ( - // envoyDaemonSetName is the name of Envoy's DaemonSet resource. - // [TODO] danehans: Remove and use contour.Name + "-envoy" when - // https://github.com/projectcontour/contour/issues/2122 is fixed. - envoyDaemonSetName = "envoy" + // envoyDaemonSetNamePrefix is the prefix of the name of Envoy's DaemonSet resource. + envoyDaemonSetNamePrefix = "envoy" // EnvoyContainerName is the name of the Envoy container. EnvoyContainerName = "envoy" // ShutdownContainerName is the name of the Shutdown Manager container. @@ -70,6 +68,11 @@ const ( xdsResourceVersion = "v3" ) +// envoyDaemonSetName returns the name of Envoy's DaemonSet resource. +func envoyDaemonSetName(contour *model.Contour) string { + return fmt.Sprintf("%s-%s", envoyDaemonSetNamePrefix, contour.Name) +} + // EnsureDaemonSet ensures a DaemonSet exists for the given contour. func EnsureDaemonSet(ctx context.Context, cli client.Client, contour *model.Contour, contourImage, envoyImage string) error { desired := DesiredDaemonSet(contour, contourImage, envoyImage) @@ -307,7 +310,7 @@ func DesiredDaemonSet(contour *model.Contour, contourImage, envoyImage string) * ds := &appsv1.DaemonSet{ ObjectMeta: metav1.ObjectMeta{ Namespace: contour.Namespace, - Name: fmt.Sprintf("%s-%s", envoyDaemonSetName, contour.Name), + Name: envoyDaemonSetName(contour), Labels: labels, }, Spec: appsv1.DaemonSetSpec{ @@ -357,7 +360,7 @@ func DesiredDaemonSet(contour *model.Contour, contourImage, envoyImage string) * }, }, }, - ServiceAccountName: fmt.Sprintf("%s-%s", objutil.EnvoyRbacName, contour.Name), + ServiceAccountName: objutil.GetEnvoyRBACNames(contour).ServiceAccount, AutomountServiceAccountToken: pointer.BoolPtr(false), TerminationGracePeriodSeconds: pointer.Int64Ptr(int64(300)), SecurityContext: objutil.NewUnprivilegedPodSecurity(), @@ -385,7 +388,7 @@ func CurrentDaemonSet(ctx context.Context, cli client.Client, contour *model.Con ds := &appsv1.DaemonSet{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: fmt.Sprintf("%s-%s", envoyDaemonSetName, contour.Name), + Name: envoyDaemonSetName(contour), } if err := cli.Get(ctx, key, ds); err != nil { return nil, err diff --git a/internal/provisioner/objects/deployment/deployment.go b/internal/provisioner/objects/deployment/deployment.go index aee333d5112..08a27fc9c98 100644 --- a/internal/provisioner/objects/deployment/deployment.go +++ b/internal/provisioner/objects/deployment/deployment.go @@ -37,10 +37,8 @@ import ( ) const ( - // contourDeploymentName is the name of Contour's Deployment resource. - // [TODO] danehans: Remove and use contour.Name + "-contour" when - // https://github.com/projectcontour/contour/issues/2122 is fixed. - contourDeploymentName = "contour" + // contourDeploymentNamePrefix is the name of Contour's Deployment resource. + contourDeploymentNamePrefix = "contour" // contourContainerName is the name of the Contour container. contourContainerName = "contour" // contourNsEnvVar is the name of the contour namespace environment variable. @@ -65,6 +63,11 @@ const ( debugPort = 6060 ) +// contourDeploymentNme returns the name of Contour's Deployment resource. +func contourDeploymentName(contour *model.Contour) string { + return fmt.Sprintf("%s-%s", contourDeploymentNamePrefix, contour.Name) +} + // EnsureDeployment ensures a deployment using image exists for the given contour. func EnsureDeployment(ctx context.Context, cli client.Client, contour *model.Contour, image string) error { desired := DesiredDeployment(contour, image) @@ -225,7 +228,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment deploy := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Namespace: contour.Namespace, - Name: fmt.Sprintf("%s-%s", contourDeploymentName, contour.Name), + Name: contourDeploymentName(contour), Labels: makeDeploymentLabels(contour), }, Spec: appsv1.DeploymentSpec{ @@ -285,7 +288,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ - Name: fmt.Sprintf("%s-%s", objcm.ContourConfigMapName, contour.Name), + Name: objcm.ContourConfigMapName(contour), }, Items: []corev1.KeyToPath{ { @@ -299,7 +302,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment }, }, DNSPolicy: corev1.DNSClusterFirst, - ServiceAccountName: fmt.Sprintf("%s-%s", objutil.ContourRbacName, contour.Name), + ServiceAccountName: objutil.GetContourRBACNames(contour).ServiceAccount, RestartPolicy: corev1.RestartPolicyAlways, SchedulerName: "default-scheduler", SecurityContext: objutil.NewUnprivilegedPodSecurity(), @@ -325,7 +328,7 @@ func CurrentDeployment(ctx context.Context, cli client.Client, contour *model.Co deploy := &appsv1.Deployment{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: fmt.Sprintf("%s-%s", contourDeploymentName, contour.Name), + Name: contourDeploymentName(contour), } if err := cli.Get(ctx, key, deploy); err != nil { return nil, err diff --git a/internal/provisioner/objects/job/job.go b/internal/provisioner/objects/job/job.go index 4bd8a6afa56..6485f6acb8b 100644 --- a/internal/provisioner/objects/job/job.go +++ b/internal/provisioner/objects/job/job.go @@ -127,7 +127,7 @@ func DesiredJob(contour *model.Contour, image string) *batchv1.Job { } spec := corev1.PodSpec{ Containers: []corev1.Container{container}, - ServiceAccountName: fmt.Sprintf("%s-%s", objutil.CertGenRbacName, contour.Name), + ServiceAccountName: objutil.GetCertgenRBACNames(contour).ServiceAccount, SecurityContext: objutil.NewUnprivilegedPodSecurity(), RestartPolicy: corev1.RestartPolicyNever, DNSPolicy: corev1.DNSClusterFirst, diff --git a/internal/provisioner/objects/rbac.go b/internal/provisioner/objects/rbac.go index b01ce015ae0..092df34ca69 100644 --- a/internal/provisioner/objects/rbac.go +++ b/internal/provisioner/objects/rbac.go @@ -26,141 +26,213 @@ import ( objsa "github.com/projectcontour/contour/internal/provisioner/objects/serviceaccount" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - utilerrors "k8s.io/apimachinery/pkg/util/errors" "sigs.k8s.io/controller-runtime/pkg/client" ) const ( - // ContourRbacName is the name used for Contour RBAC resources. - ContourRbacName = "contour" - // ContourRoleBindingName is a special case RoleBinding name since + // contourRbacNamePrefix is the prefix of the name used for Contour RBAC resources. + contourRbacNamePrefix = "contour" + + // contourRoleBindingNamePrefix is a special case RoleBinding name prefix since // the certgen RoleBinding name is "contour" - ContourRoleBindingName = "contour-rolebinding" - // EnvoyRbacName is the name used for Envoy RBAC resources. - EnvoyRbacName = "envoy" - // CertGenRbacName is the name used for Contour certificate + contourRoleBindingNamePrefix = "contour-rolebinding" + + // envoyRbacNamePrefix is the prefix of the name used for Envoy RBAC resources. + envoyRbacNamePrefix = "envoy" + + // certGenRbacNamePrefix is the prefix of the name used for Contour certificate // generation RBAC resources. - CertGenRbacName = "contour-certgen" + certGenRbacNamePrefix = "contour-certgen" ) -// EnsureRBAC ensures all the necessary RBAC resources exist for the -// provided contour. -func EnsureRBAC(ctx context.Context, cli client.Client, contour *model.Contour) error { - ns := contour.Namespace - names := []string{ContourRbacName, EnvoyRbacName, CertGenRbacName} - for _, name := range names { - saName := fmt.Sprintf("%s-%s", name, contour.Name) - - _, err := objsa.EnsureServiceAccount(ctx, cli, saName, contour) - if err != nil { - return fmt.Errorf("failed to ensure service account %s/%s: %w", ns, saName, err) - } - } - // ClusterRole and ClusterRoleBinding resources are namespace-named to allow ownership - // from individual instances of Contour. - nsName := fmt.Sprintf("%s-%s-%s", ContourRbacName, ns, contour.Name) - cr, err := objcr.EnsureClusterRole(ctx, cli, nsName, contour) - if err != nil { - return fmt.Errorf("failed to ensure cluster role %s: %w", nsName, err) +// RBACNames holds a set of names of related RBAC resources. +type RBACNames struct { + ServiceAccount string + ClusterRole string + ClusterRoleBinding string + Role string + RoleBinding string +} + +// GetContourRBACNames returns the names of the RBAC resources for +// the Contour deployment. +func GetContourRBACNames(contour *model.Contour) RBACNames { + return RBACNames{ + ServiceAccount: fmt.Sprintf("%s-%s", contourRbacNamePrefix, contour.Name), + ClusterRole: fmt.Sprintf("%s-%s-%s", contourRbacNamePrefix, contour.Namespace, contour.Name), + ClusterRoleBinding: fmt.Sprintf("%s-%s-%s", contourRbacNamePrefix, contour.Namespace, contour.Name), + Role: fmt.Sprintf("%s-%s", contourRbacNamePrefix, contour.Name), + + // this one has a different prefix to differentiate from the certgen role binding (see below). + RoleBinding: fmt.Sprintf("%s-%s", contourRoleBindingNamePrefix, contour.Name), } - if err := objcrb.EnsureClusterRoleBinding(ctx, cli, nsName, cr.Name, fmt.Sprintf("%s-%s", ContourRbacName, contour.Name), contour); err != nil { - return fmt.Errorf("failed to ensure cluster role binding %s: %w", fmt.Sprintf("%s-%s", ContourRbacName, contour.Name), err) +} + +// GetEnvoyRBACNames returns the names of the RBAC resources for +// the Envoy daemonset. +func GetEnvoyRBACNames(contour *model.Contour) RBACNames { + return RBACNames{ + ServiceAccount: fmt.Sprintf("%s-%s", envoyRbacNamePrefix, contour.Name), } - certRole, err := objrole.EnsureCertgenRole(ctx, cli, fmt.Sprintf("%s-%s", CertGenRbacName, contour.Name), contour) - if err != nil { - return fmt.Errorf("failed to ensure certgen role %s/%s: %w", ns, fmt.Sprintf("%s-%s", CertGenRbacName, contour.Name), err) +} + +// GetCertgenRBACNames returns the names of the RBAC resources for +// the Certgen job. +func GetCertgenRBACNames(contour *model.Contour) RBACNames { + return RBACNames{ + ServiceAccount: fmt.Sprintf("%s-%s", certGenRbacNamePrefix, contour.Name), + Role: fmt.Sprintf("%s-%s", certGenRbacNamePrefix, contour.Name), + + // this one is name contour- despite being for certgen for legacy reasons. + RoleBinding: fmt.Sprintf("%s-%s", contourRbacNamePrefix, contour.Name), } - // this one is named "contour-" despite being for certgen. - if err := objrb.EnsureRoleBinding(ctx, cli, fmt.Sprintf("%s-%s", ContourRbacName, contour.Name), fmt.Sprintf("%s-%s", CertGenRbacName, contour.Name), certRole.Name, contour); err != nil { - return fmt.Errorf("failed to ensure certgen role binding %s/%s: %w", ns, ContourRbacName, err) +} + +// EnsureRBAC ensures all the necessary RBAC resources exist for the +// provided contour. +func EnsureRBAC(ctx context.Context, cli client.Client, contour *model.Contour) error { + if err := ensureContourRBAC(ctx, cli, contour); err != nil { + return fmt.Errorf("failed to ensure Contour RBAC: %w", err) } - controllerRole, err := objrole.EnsureControllerRole(ctx, cli, fmt.Sprintf("%s-%s", ContourRbacName, contour.Name), contour) - if err != nil { - return fmt.Errorf("failed to ensure controller role %s/%s: %w", ns, fmt.Sprintf("%s-%s", ContourRbacName, contour.Name), err) + + if err := ensureEnvoyRBAC(ctx, cli, contour); err != nil { + return fmt.Errorf("failed to ensure Envoy RBAC: %w", err) } - if err := objrb.EnsureRoleBinding(ctx, cli, fmt.Sprintf("%s-%s", ContourRoleBindingName, contour.Name), fmt.Sprintf("%s-%s", ContourRbacName, contour.Name), controllerRole.Name, contour); err != nil { - return fmt.Errorf("failed to ensure controller role binding %s/%s: %w", ns, fmt.Sprintf("%s-%s", ContourRoleBindingName, contour.Name), err) + + if err := ensureCertgenRBAC(ctx, cli, contour); err != nil { + return fmt.Errorf("failed to ensure Certgen RBAC: %w", err) } + return nil } -// EnsureRBACDeleted ensures all the necessary RBAC resources for the provided -// contour are deleted if Contour owner labels exist. -func EnsureRBACDeleted(ctx context.Context, cli client.Client, contour *model.Contour) error { - var errs []error - ns := contour.Namespace - objectsToDelete := []client.Object{} +func ensureContourRBAC(ctx context.Context, cli client.Client, contour *model.Contour) error { + names := GetContourRBACNames(contour) + + // Ensure service account. + if _, err := objsa.EnsureServiceAccount(ctx, cli, names.ServiceAccount, contour); err != nil { + return fmt.Errorf("failed to ensure service account %s/%s: %w", contour.Namespace, names.ServiceAccount, err) + } - controllerRoleBind, err := objrb.CurrentRoleBinding(ctx, cli, ns, ContourRoleBindingName) - if err != nil && !errors.IsNotFound(err) { - return err + // Ensure cluster role & binding. + if _, err := objcr.EnsureClusterRole(ctx, cli, names.ClusterRole, contour); err != nil { + return fmt.Errorf("failed to ensure cluster role %s: %w", names.ClusterRole, err) } - if controllerRoleBind != nil { - objectsToDelete = append(objectsToDelete, controllerRoleBind) + if err := objcrb.EnsureClusterRoleBinding(ctx, cli, names.ClusterRoleBinding, names.ClusterRole, names.ServiceAccount, contour); err != nil { + return fmt.Errorf("failed to ensure cluster role binding %s: %w", names.ClusterRoleBinding, err) } - controllerRole, err := objrole.CurrentRole(ctx, cli, ns, ContourRbacName) - if err != nil && !errors.IsNotFound(err) { - return err + + // Ensure role & binding. + if _, err := objrole.EnsureControllerRole(ctx, cli, names.Role, contour); err != nil { + return fmt.Errorf("failed to ensure controller role %s/%s: %w", contour.Namespace, names.Role, err) } - if controllerRole != nil { - objectsToDelete = append(objectsToDelete, controllerRole) + if err := objrb.EnsureRoleBinding(ctx, cli, names.RoleBinding, names.ServiceAccount, names.Role, contour); err != nil { + return fmt.Errorf("failed to ensure controller role binding %s/%s: %w", contour.Namespace, names.RoleBinding, err) } - certRoleBind, err := objrb.CurrentRoleBinding(ctx, cli, ns, ContourRbacName) - if err != nil && !errors.IsNotFound(err) { - return err + + return nil +} + +func ensureEnvoyRBAC(ctx context.Context, cli client.Client, contour *model.Contour) error { + names := GetEnvoyRBACNames(contour) + + // Ensure service account. + if _, err := objsa.EnsureServiceAccount(ctx, cli, names.ServiceAccount, contour); err != nil { + return fmt.Errorf("failed to ensure service account %s/%s: %w", contour.Namespace, names.ServiceAccount, err) } - if certRoleBind != nil { - objectsToDelete = append(objectsToDelete, certRoleBind) + + return nil +} + +func ensureCertgenRBAC(ctx context.Context, cli client.Client, contour *model.Contour) error { + names := GetCertgenRBACNames(contour) + + // Ensure service account. + if _, err := objsa.EnsureServiceAccount(ctx, cli, names.ServiceAccount, contour); err != nil { + return fmt.Errorf("failed to ensure service account %s/%s: %w", contour.Namespace, names.ServiceAccount, err) } - certRole, err := objrole.CurrentRole(ctx, cli, ns, CertGenRbacName) - if err != nil && !errors.IsNotFound(err) { - return err + + // Ensure role & binding. + if _, err := objrole.EnsureCertgenRole(ctx, cli, names.Role, contour); err != nil { + return fmt.Errorf("failed to ensure certgen role %s/%s: %w", contour.Namespace, names.Role, err) } - if certRole != nil { - objectsToDelete = append(objectsToDelete, certRole) + if err := objrb.EnsureRoleBinding(ctx, cli, names.RoleBinding, names.ServiceAccount, names.Role, contour); err != nil { + return fmt.Errorf("failed to ensure certgen role binding %s/%s: %w", contour.Namespace, names.RoleBinding, err) } - names := []string{ContourRbacName, EnvoyRbacName, CertGenRbacName} - for _, name := range names { - svcAct, err := objsa.CurrentServiceAccount(ctx, cli, ns, name) - if err != nil && !errors.IsNotFound(err) { - return err + + return nil +} + +// EnsureRBACDeleted ensures all the necessary RBAC resources for the provided +// contour are deleted if Contour owner labels exist. +func EnsureRBACDeleted(ctx context.Context, cli client.Client, contour *model.Contour) error { + var deletions []client.Object + + for _, name := range []RBACNames{ + GetContourRBACNames(contour), + GetEnvoyRBACNames(contour), + GetCertgenRBACNames(contour), + } { + if len(name.RoleBinding) > 0 { + rolebinding, err := objrb.CurrentRoleBinding(ctx, cli, contour.Namespace, name.RoleBinding) + if err != nil && !errors.IsNotFound(err) { + return err + } + if rolebinding != nil { + deletions = append(deletions, rolebinding) + } } - if svcAct != nil { - objectsToDelete = append(objectsToDelete, svcAct) + + if len(name.Role) > 0 { + role, err := objrole.CurrentRole(ctx, cli, contour.Namespace, name.Role) + if err != nil && !errors.IsNotFound(err) { + return err + } + if role != nil { + deletions = append(deletions, role) + } } - } - // ClusterRole and ClusterRoleBinding resources are namespace-named to allow ownership - // from individual instances of Contour. - nsName := fmt.Sprintf("%s-%s", ContourRbacName, contour.Namespace) - crb, err := objcrb.CurrentClusterRoleBinding(ctx, cli, nsName) - if err != nil && !errors.IsNotFound(err) { - return err - } - if crb != nil { - objectsToDelete = append(objectsToDelete, crb) - } - cr, err := objcr.CurrentClusterRole(ctx, cli, nsName) - if err != nil && !errors.IsNotFound(err) { - return err - } - if cr != nil { - objectsToDelete = append(objectsToDelete, cr) - } + if len(name.ClusterRoleBinding) > 0 { + clusterrolebinding, err := objcrb.CurrentClusterRoleBinding(ctx, cli, name.ClusterRoleBinding) + if err != nil && !errors.IsNotFound(err) { + return err + } + if clusterrolebinding != nil { + deletions = append(deletions, clusterrolebinding) + } + } + + if len(name.ClusterRole) > 0 { + clusterrole, err := objcr.CurrentClusterRole(ctx, cli, name.ClusterRole) + if err != nil && !errors.IsNotFound(err) { + return err + } + if clusterrole != nil { + deletions = append(deletions, clusterrole) + } + } - for _, object := range objectsToDelete { - kind := object.GetObjectKind().GroupVersionKind().Kind - namespace := object.(metav1.Object).GetNamespace() - name := object.(metav1.Object).GetName() - if labels.Exist(object, model.OwnerLabels(contour)) { - if err := cli.Delete(ctx, object); err != nil { - if errors.IsNotFound(err) { - continue - } - return fmt.Errorf("failed to delete %s %s/%s: %w", kind, namespace, name, err) + if len(name.ServiceAccount) > 0 { + serviceaccount, err := objsa.CurrentServiceAccount(ctx, cli, contour.Namespace, name.ServiceAccount) + if err != nil && !errors.IsNotFound(err) { + return err + } + if serviceaccount != nil { + deletions = append(deletions, serviceaccount) } } } - return utilerrors.NewAggregate(errs) + + for _, deletion := range deletions { + if !labels.Exist(deletion, model.OwnerLabels(contour)) { + continue + } + + if err := cli.Delete(ctx, deletion); err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("failed to delete %s %s/%s: %w", deletion.GetObjectKind().GroupVersionKind().Kind, deletion.GetNamespace(), deletion.GetName(), err) + } + } + + return nil } diff --git a/internal/provisioner/objects/service/service.go b/internal/provisioner/objects/service/service.go index c4fb5af041b..1bd6ff6ad0d 100644 --- a/internal/provisioner/objects/service/service.go +++ b/internal/provisioner/objects/service/service.go @@ -34,14 +34,10 @@ import ( ) const ( - // contourSvcName is the name of Contour's Service. - // [TODO] danehans: Update Contour name to contour.Name + "-contour" to support multiple - // Contours/ns when https://github.com/projectcontour/contour/issues/2122 is fixed. - contourSvcName = "contour" - // [TODO] danehans: Update Envoy name to contour.Name + "-envoy" to support multiple - // Contours/ns when https://github.com/projectcontour/contour/issues/2122 is fixed. - // envoySvcName is the name of Envoy's Service. - envoySvcName = "envoy" + // contourSvcNamePrefix is the prefix of the name of Contour's Service. + contourSvcNamePrefix = "contour" + // envoySvcNamePrefix is the prefix of the name of Envoy's Service. + envoySvcNamePrefix = "envoy" // awsLbBackendProtoAnnotation is a Service annotation that places the AWS ELB into // "TCP" mode so that it does not do HTTP negotiation for HTTPS connections at the // ELB edge. The downside of this is the remote IP address of all connections will @@ -95,6 +91,16 @@ const ( EnvoyNodePortHTTPSPort = int32(30443) ) +// contourServiceName returns the name of Contour's Service resource. +func contourServiceName(contour *model.Contour) string { + return fmt.Sprintf("%s-%s", contourSvcNamePrefix, contour.Name) +} + +// envoyServiceName returns the name of Envoy's service resource. +func envoyServiceName(contour *model.Contour) string { + return fmt.Sprintf("%s-%s", envoySvcNamePrefix, contour.Name) +} + var ( // InternalLBAnnotations maps cloud providers to the provider's annotation // key/value pair used for managing an internal load balancer. For additional @@ -196,7 +202,7 @@ func DesiredContourService(contour *model.Contour) *corev1.Service { svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Namespace: contour.Namespace, - Name: fmt.Sprintf("%s-%s", contourSvcName, contour.Name), + Name: contourServiceName(contour), Labels: model.OwnerLabels(contour), }, Spec: corev1.ServiceSpec{ @@ -264,7 +270,7 @@ func DesiredEnvoyService(contour *model.Contour) *corev1.Service { svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Namespace: contour.Namespace, - Name: fmt.Sprintf("%s-%s", envoySvcName, contour.Name), + Name: envoyServiceName(contour), Annotations: map[string]string{}, Labels: model.OwnerLabels(contour), }, @@ -356,7 +362,7 @@ func currentContourService(ctx context.Context, cli client.Client, contour *mode current := &corev1.Service{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: fmt.Sprintf("%s-%s", contourSvcName, contour.Name), + Name: contourServiceName(contour), } err := cli.Get(ctx, key, current) if err != nil { @@ -370,7 +376,7 @@ func currentEnvoyService(ctx context.Context, cli client.Client, contour *model. current := &corev1.Service{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: fmt.Sprintf("%s-%s", envoySvcName, contour.Name), + Name: envoyServiceName(contour), } err := cli.Get(ctx, key, current) if err != nil { From 1ec9d552bec01da7266db84419166bcc8f1ab2dd Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Tue, 22 Mar 2022 22:39:32 +0000 Subject: [PATCH 4/6] multiple gateways per namespace E2E Signed-off-by: Steve Kriss --- test/e2e/provisioner/provisioner_test.go | 54 ++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/e2e/provisioner/provisioner_test.go b/test/e2e/provisioner/provisioner_test.go index a159d425df2..db740aff13d 100644 --- a/test/e2e/provisioner/provisioner_test.go +++ b/test/e2e/provisioner/provisioner_test.go @@ -95,6 +95,60 @@ var _ = Describe("Gateway provisioner", func() { require.True(f.T(), ok) }) }) + + f.NamespacedTest("multiple-gateways-per-namespace", func(namespace string) { + Specify("Multiple basic one-listener HTTP gateways can be provisioned in a single namespace", func() { + gateway := &gatewayapi_v1alpha2.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "http-1", + Namespace: namespace, + }, + Spec: gatewayapi_v1alpha2.GatewaySpec{ + GatewayClassName: gatewayapi_v1alpha2.ObjectName("contour"), + Listeners: []gatewayapi_v1alpha2.Listener{ + { + Name: "http", + Protocol: gatewayapi_v1alpha2.HTTPProtocolType, + Port: gatewayapi_v1alpha2.PortNumber(80), + AllowedRoutes: &gatewayapi_v1alpha2.AllowedRoutes{ + Namespaces: &gatewayapi_v1alpha2.RouteNamespaces{ + From: gatewayapi.FromNamespacesPtr(gatewayapi_v1alpha2.NamespacesFromSame), + }, + }, + }, + }, + }, + } + + _, ok := f.CreateGatewayAndWaitFor(gateway, gatewayReady) + require.True(f.T(), ok) + + gateway = &gatewayapi_v1alpha2.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "http-2", + Namespace: namespace, + }, + Spec: gatewayapi_v1alpha2.GatewaySpec{ + GatewayClassName: gatewayapi_v1alpha2.ObjectName("contour"), + Listeners: []gatewayapi_v1alpha2.Listener{ + { + Name: "http", + Protocol: gatewayapi_v1alpha2.HTTPProtocolType, + Port: gatewayapi_v1alpha2.PortNumber(80), + AllowedRoutes: &gatewayapi_v1alpha2.AllowedRoutes{ + Namespaces: &gatewayapi_v1alpha2.RouteNamespaces{ + From: gatewayapi.FromNamespacesPtr(gatewayapi_v1alpha2.NamespacesFromSame), + }, + }, + }, + }, + }, + } + + _, ok = f.CreateGatewayAndWaitFor(gateway, gatewayReady) + require.True(f.T(), ok) + }) + }) }) // gatewayClassAccepted returns true if the gateway has a .status.conditions From 9b4c5655f669afe77e644ac3ccb8454fe0b85d07 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Tue, 29 Mar 2022 15:20:16 +0000 Subject: [PATCH 5/6] move name generation into model, use everywhere Signed-off-by: Steve Kriss --- internal/provisioner/model/names.go | 117 ++++++++++++++++++ .../objects/configmap/configmap.go | 9 +- .../objects/daemonset/daemonset.go | 27 ++-- .../objects/deployment/deployment.go | 34 ++--- internal/provisioner/objects/job/job.go | 10 +- internal/provisioner/objects/object.go | 12 -- internal/provisioner/objects/rbac.go | 73 ++--------- .../provisioner/objects/service/service.go | 26 +--- 8 files changed, 156 insertions(+), 152 deletions(-) create mode 100644 internal/provisioner/model/names.go diff --git a/internal/provisioner/model/names.go b/internal/provisioner/model/names.go new file mode 100644 index 00000000000..37b564922c4 --- /dev/null +++ b/internal/provisioner/model/names.go @@ -0,0 +1,117 @@ +// Copyright Project Contour Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package model + +import ( + "fmt" + "strings" +) + +// ConfigMapName returns the name of the Contour ConfigMap resource. +func (c *Contour) ConfigMapName() string { + return "contour-" + c.Name +} + +// ContourServiceName returns the name of the Contour Service resource. +func (c *Contour) ContourServiceName() string { + return "contour-" + c.Name +} + +// EnvoyServiceName returns the name of the Envoy Service resource. +func (c *Contour) EnvoyServiceName() string { + return "envoy-" + c.Name +} + +// ContourDeploymentName returns the name of the Contour Deployment resource. +func (c *Contour) ContourDeploymentName() string { + return "contour-" + c.Name +} + +// EnvoyDaemonSetName returns the name of the Envoy DaemonSet resource. +func (c *Contour) EnvoyDaemonSetName() string { + return "envoy-" + c.Name +} + +// LeaderElectionLeaseName returns the name of the Contour leader election Lease resource. +func (c *Contour) LeaderElectionLeaseName() string { + return "leader-elect-" + c.Name +} + +// ContourCertsSecretName returns the name of the Contour xDS TLS certs Secret resource. +func (c *Contour) ContourCertsSecretName() string { + return c.Name + "-contourcert" +} + +// EnvoyCertsSecretName returns the name of the Envoy xDS TLS certs Secret resource. +func (c *Contour) EnvoyCertsSecretName() string { + return c.Name + "-envoycert" +} + +// CertgenJobName returns the name of the certgen Job resource. +func (c *Contour) CertgenJobName(contourImage string) string { + return fmt.Sprintf("contour-certgen-%s-%s", tagFromImage(contourImage), c.Name) +} + +// tagFromImage returns the tag from the provided image or an +// empty string if the image does not contain a tag. +func tagFromImage(image string) string { + if strings.Contains(image, ":") { + parsed := strings.Split(image, ":") + return parsed[1] + } + return "" +} + +// ContourRBACNames returns the names of the RBAC resources for +// the Contour deployment. +func (c *Contour) ContourRBACNames() RBACNames { + return RBACNames{ + ServiceAccount: fmt.Sprintf("contour-%s", c.Name), + ClusterRole: fmt.Sprintf("contour-%s-%s", c.Namespace, c.Name), + ClusterRoleBinding: fmt.Sprintf("contour-%s-%s", c.Namespace, c.Name), + Role: fmt.Sprintf("contour-%s", c.Name), + + // this one has a different prefix to differentiate from the certgen role binding (see below). + RoleBinding: fmt.Sprintf("contour-rolebinding-%s", c.Name), + } +} + +// EnvoyRBACNames returns the names of the RBAC resources for +// the Envoy daemonset. +func (c *Contour) EnvoyRBACNames() RBACNames { + return RBACNames{ + ServiceAccount: "envoy-" + c.Name, + } +} + +// CertgenRBACNames returns the names of the RBAC resources for +// the Certgen job. +func (c *Contour) CertgenRBACNames() RBACNames { + return RBACNames{ + ServiceAccount: "contour-certgen-" + c.Name, + Role: "contour-certgen-" + c.Name, + + // this one is name contour- despite being for certgen for legacy reasons. + RoleBinding: "contour-" + c.Name, + } +} + +// RBACNames holds a set of names of related RBAC resources. +type RBACNames struct { + ServiceAccount string + ClusterRole string + ClusterRoleBinding string + Role string + RoleBinding string +} diff --git a/internal/provisioner/objects/configmap/configmap.go b/internal/provisioner/objects/configmap/configmap.go index 755d56dfd23..8961a68785e 100644 --- a/internal/provisioner/objects/configmap/configmap.go +++ b/internal/provisioner/objects/configmap/configmap.go @@ -31,11 +31,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// ContourConfigMapName returns the name of Contour's ConfigMap resource. -func ContourConfigMapName(contour *model.Contour) string { - return fmt.Sprintf("%s-%s", "contour", contour.Name) -} - var contourConfigMapTemplate = template.Must(template.New("contour.yaml").Parse(`# # server: # determine which XDS Server implementation to utilize in Contour. @@ -187,13 +182,13 @@ type contourConfig struct { func configForContour(contour *model.Contour) *configMapParams { return &configMapParams{ Namespace: contour.Namespace, - Name: ContourConfigMapName(contour), + Name: contour.ConfigMapName(), Labels: model.OwnerLabels(contour), Contour: contourConfig{ GatewayNamespace: contour.Namespace, GatewayName: contour.Name, EnableExternalNameService: pointer.BoolDeref(contour.Spec.EnableExternalNameService, false), - EnvoyServiceName: fmt.Sprintf("envoy-%s", contour.Name), + EnvoyServiceName: contour.EnvoyServiceName(), }, } } diff --git a/internal/provisioner/objects/daemonset/daemonset.go b/internal/provisioner/objects/daemonset/daemonset.go index 125e4fc6482..6855e20e4ba 100644 --- a/internal/provisioner/objects/daemonset/daemonset.go +++ b/internal/provisioner/objects/daemonset/daemonset.go @@ -36,8 +36,6 @@ import ( ) const ( - // envoyDaemonSetNamePrefix is the prefix of the name of Envoy's DaemonSet resource. - envoyDaemonSetNamePrefix = "envoy" // EnvoyContainerName is the name of the Envoy container. EnvoyContainerName = "envoy" // ShutdownContainerName is the name of the Shutdown Manager container. @@ -52,8 +50,6 @@ const ( envoyCertsVolName = "envoycert" // envoyCertsVolMntDir is the directory name of the Envoy certificates volume. envoyCertsVolMntDir = "certs" - // envoyCertsSecretName is the name of the secret used as the certificate volume source. - envoyCertsSecretName = envoyCertsVolName // envoyCfgVolName is the name of the Envoy configuration volume. envoyCfgVolName = "envoy-config" // envoyCfgVolMntDir is the directory name of the Envoy configuration volume. @@ -68,11 +64,6 @@ const ( xdsResourceVersion = "v3" ) -// envoyDaemonSetName returns the name of Envoy's DaemonSet resource. -func envoyDaemonSetName(contour *model.Contour) string { - return fmt.Sprintf("%s-%s", envoyDaemonSetNamePrefix, contour.Name) -} - // EnsureDaemonSet ensures a DaemonSet exists for the given contour. func EnsureDaemonSet(ctx context.Context, cli client.Client, contour *model.Contour, contourImage, envoyImage string) error { desired := DesiredDaemonSet(contour, contourImage, envoyImage) @@ -271,7 +262,7 @@ func DesiredDaemonSet(contour *model.Contour, contourImage, envoyImage string) * Args: []string{ "bootstrap", filepath.Join("/", envoyCfgVolMntDir, envoyCfgFileName), - "--xds-address=contour-" + contour.Name, + fmt.Sprintf("--xds-address=%s", contour.ContourServiceName()), fmt.Sprintf("--xds-port=%d", objcfg.XDSPort), fmt.Sprintf("--xds-resource-version=%s", xdsResourceVersion), fmt.Sprintf("--resources-dir=%s", filepath.Join("/", envoyCfgVolMntDir, "resources")), @@ -310,13 +301,13 @@ func DesiredDaemonSet(contour *model.Contour, contourImage, envoyImage string) * ds := &appsv1.DaemonSet{ ObjectMeta: metav1.ObjectMeta{ Namespace: contour.Namespace, - Name: envoyDaemonSetName(contour), + Name: contour.EnvoyDaemonSetName(), Labels: labels, }, Spec: appsv1.DaemonSetSpec{ RevisionHistoryLimit: pointer.Int32Ptr(int32(10)), // Ensure the deamonset adopts only its own pods. - Selector: EnvoyDaemonSetPodSelector(contour.Name), + Selector: EnvoyDaemonSetPodSelector(contour), UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ Type: appsv1.RollingUpdateDaemonSetStrategyType, RollingUpdate: &appsv1.RollingUpdateDaemonSet{ @@ -332,7 +323,7 @@ func DesiredDaemonSet(contour *model.Contour, contourImage, envoyImage string) * "prometheus.io/port": "8002", "prometheus.io/path": "/stats/prometheus", }, - Labels: EnvoyDaemonSetPodSelector(contour.Name).MatchLabels, + Labels: EnvoyDaemonSetPodSelector(contour).MatchLabels, }, Spec: corev1.PodSpec{ Containers: containers, @@ -343,7 +334,7 @@ func DesiredDaemonSet(contour *model.Contour, contourImage, envoyImage string) * VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ DefaultMode: pointer.Int32Ptr(int32(420)), - SecretName: fmt.Sprintf("%s-%s", contour.Name, envoyCertsSecretName), + SecretName: contour.EnvoyCertsSecretName(), }, }, }, @@ -360,7 +351,7 @@ func DesiredDaemonSet(contour *model.Contour, contourImage, envoyImage string) * }, }, }, - ServiceAccountName: objutil.GetEnvoyRBACNames(contour).ServiceAccount, + ServiceAccountName: contour.EnvoyRBACNames().ServiceAccount, AutomountServiceAccountToken: pointer.BoolPtr(false), TerminationGracePeriodSeconds: pointer.Int64Ptr(int64(300)), SecurityContext: objutil.NewUnprivilegedPodSecurity(), @@ -388,7 +379,7 @@ func CurrentDaemonSet(ctx context.Context, cli client.Client, contour *model.Con ds := &appsv1.DaemonSet{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: envoyDaemonSetName(contour), + Name: contour.EnvoyDaemonSetName(), } if err := cli.Get(ctx, key, ds); err != nil { return nil, err @@ -421,10 +412,10 @@ func updateDaemonSetIfNeeded(ctx context.Context, cli client.Client, contour *mo // EnvoyDaemonSetPodSelector returns a label selector using "app: envoy" as the // key/value pair. -func EnvoyDaemonSetPodSelector(contourName string) *metav1.LabelSelector { +func EnvoyDaemonSetPodSelector(contour *model.Contour) *metav1.LabelSelector { return &metav1.LabelSelector{ MatchLabels: map[string]string{ - "app": "envoy-" + contourName, + "app": contour.EnvoyDaemonSetName(), }, } } diff --git a/internal/provisioner/objects/deployment/deployment.go b/internal/provisioner/objects/deployment/deployment.go index 08a27fc9c98..c48c59cc43f 100644 --- a/internal/provisioner/objects/deployment/deployment.go +++ b/internal/provisioner/objects/deployment/deployment.go @@ -23,7 +23,6 @@ import ( "github.com/projectcontour/contour/internal/provisioner/labels" "github.com/projectcontour/contour/internal/provisioner/model" objutil "github.com/projectcontour/contour/internal/provisioner/objects" - objcm "github.com/projectcontour/contour/internal/provisioner/objects/configmap" objcfg "github.com/projectcontour/contour/internal/provisioner/objects/sharedconfig" appsv1 "k8s.io/api/apps/v1" @@ -37,8 +36,6 @@ import ( ) const ( - // contourDeploymentNamePrefix is the name of Contour's Deployment resource. - contourDeploymentNamePrefix = "contour" // contourContainerName is the name of the Contour container. contourContainerName = "contour" // contourNsEnvVar is the name of the contour namespace environment variable. @@ -49,8 +46,6 @@ const ( contourCertsVolName = "contourcert" // contourCertsVolMntDir is the directory name of the contour certificates volume. contourCertsVolMntDir = "certs" - // contourCertsSecretName is the name of the secret used as the certificate volume source. - contourCertsSecretName = contourCertsVolName // contourCfgVolName is the name of the contour configuration volume. contourCfgVolName = "contour-config" // contourCfgVolMntDir is the directory name of the contour configuration volume. @@ -63,11 +58,6 @@ const ( debugPort = 6060 ) -// contourDeploymentNme returns the name of Contour's Deployment resource. -func contourDeploymentName(contour *model.Contour) string { - return fmt.Sprintf("%s-%s", contourDeploymentNamePrefix, contour.Name) -} - // EnsureDeployment ensures a deployment using image exists for the given contour. func EnsureDeployment(ctx context.Context, cli client.Client, contour *model.Contour, image string) error { desired := DesiredDeployment(contour, image) @@ -125,8 +115,8 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment fmt.Sprintf("--contour-cert-file=%s", filepath.Join("/", contourCertsVolMntDir, "tls.crt")), fmt.Sprintf("--contour-key-file=%s", filepath.Join("/", contourCertsVolMntDir, "tls.key")), fmt.Sprintf("--config-path=%s", filepath.Join("/", contourCfgVolMntDir, contourCfgFileName)), - fmt.Sprintf("--leader-election-resource-name=%s", "leader-elect-"+contour.Name), - fmt.Sprintf("--envoy-service-name=%s", "envoy-"+contour.Name), + fmt.Sprintf("--leader-election-resource-name=%s", contour.LeaderElectionLeaseName()), + fmt.Sprintf("--envoy-service-name=%s", contour.EnvoyServiceName()), } // Pass the insecure/secure flags to Contour if using non-default ports. for _, port := range contour.Spec.NetworkPublishing.Envoy.ContainerPorts { @@ -228,7 +218,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment deploy := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Namespace: contour.Namespace, - Name: contourDeploymentName(contour), + Name: contour.ContourDeploymentName(), Labels: makeDeploymentLabels(contour), }, Spec: appsv1.DeploymentSpec{ @@ -236,7 +226,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment Replicas: &contour.Spec.Replicas, RevisionHistoryLimit: pointer.Int32Ptr(int32(10)), // Ensure the deployment adopts only its own pods. - Selector: ContourDeploymentPodSelector(contour.Name), + Selector: ContourDeploymentPodSelector(contour), Strategy: appsv1.DeploymentStrategy{ Type: appsv1.RollingUpdateDeploymentStrategyType, RollingUpdate: &appsv1.RollingUpdateDeployment{ @@ -252,7 +242,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment "prometheus.io/scrape": "true", "prometheus.io/port": fmt.Sprintf("%d", metricsPort), }, - Labels: ContourDeploymentPodSelector(contour.Name).MatchLabels, + Labels: ContourDeploymentPodSelector(contour).MatchLabels, }, Spec: corev1.PodSpec{ // TODO [danehans]: Readdress anti-affinity when https://github.com/projectcontour/contour/issues/2997 @@ -265,7 +255,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment PodAffinityTerm: corev1.PodAffinityTerm{ TopologyKey: "kubernetes.io/hostname", LabelSelector: &metav1.LabelSelector{ - MatchLabels: ContourDeploymentPodSelector(contour.Name).MatchLabels, + MatchLabels: ContourDeploymentPodSelector(contour).MatchLabels, }, }, }, @@ -279,7 +269,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ DefaultMode: pointer.Int32Ptr(int32(420)), - SecretName: fmt.Sprintf("%s-%s", contour.Name, contourCertsSecretName), + SecretName: contour.ContourCertsSecretName(), }, }, }, @@ -288,7 +278,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ - Name: objcm.ContourConfigMapName(contour), + Name: contour.ConfigMapName(), }, Items: []corev1.KeyToPath{ { @@ -302,7 +292,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment }, }, DNSPolicy: corev1.DNSClusterFirst, - ServiceAccountName: objutil.GetContourRBACNames(contour).ServiceAccount, + ServiceAccountName: contour.ContourRBACNames().ServiceAccount, RestartPolicy: corev1.RestartPolicyAlways, SchedulerName: "default-scheduler", SecurityContext: objutil.NewUnprivilegedPodSecurity(), @@ -328,7 +318,7 @@ func CurrentDeployment(ctx context.Context, cli client.Client, contour *model.Co deploy := &appsv1.Deployment{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: contourDeploymentName(contour), + Name: contour.ContourDeploymentName(), } if err := cli.Get(ctx, key, deploy); err != nil { return nil, err @@ -377,10 +367,10 @@ func makeDeploymentLabels(contour *model.Contour) map[string]string { // ContourDeploymentPodSelector returns a label selector using "app: contour" as the // key/value pair. -func ContourDeploymentPodSelector(contourName string) *metav1.LabelSelector { +func ContourDeploymentPodSelector(contour *model.Contour) *metav1.LabelSelector { return &metav1.LabelSelector{ MatchLabels: map[string]string{ - "app": "contour-" + contourName, + "app": contour.ContourDeploymentName(), }, } } diff --git a/internal/provisioner/objects/job/job.go b/internal/provisioner/objects/job/job.go index 6485f6acb8b..f720806b355 100644 --- a/internal/provisioner/objects/job/job.go +++ b/internal/provisioner/objects/job/job.go @@ -38,10 +38,6 @@ const ( jobNsEnvVar = "CONTOUR_NAMESPACE" ) -func certgenJobName(contourImage, contourName string) string { - return fmt.Sprintf("%s-%s-%s", "contour-certgen", objutil.TagFromImage(contourImage), contourName) -} - // EnsureJob ensures that a Job exists for the given contour. // TODO [danehans]: The real dependency is whether the TLS secrets are present. // The method should first check for the secrets, then use certgen as a secret @@ -87,7 +83,7 @@ func currentJob(ctx context.Context, cli client.Client, contour *model.Contour, current := &batchv1.Job{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: certgenJobName(image, contour.Name), + Name: contour.CertgenJobName(image), } err := cli.Get(ctx, key, current) if err != nil { @@ -127,7 +123,7 @@ func DesiredJob(contour *model.Contour, image string) *batchv1.Job { } spec := corev1.PodSpec{ Containers: []corev1.Container{container}, - ServiceAccountName: objutil.GetCertgenRBACNames(contour).ServiceAccount, + ServiceAccountName: contour.CertgenRBACNames().ServiceAccount, SecurityContext: objutil.NewUnprivilegedPodSecurity(), RestartPolicy: corev1.RestartPolicyNever, DNSPolicy: corev1.DNSClusterFirst, @@ -150,7 +146,7 @@ func DesiredJob(contour *model.Contour, image string) *batchv1.Job { job := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ - Name: certgenJobName(image, contour.Name), + Name: contour.CertgenJobName(image), Namespace: contour.Namespace, Labels: labels, }, diff --git a/internal/provisioner/objects/object.go b/internal/provisioner/objects/object.go index c03b0afd18b..4462a1cb767 100644 --- a/internal/provisioner/objects/object.go +++ b/internal/provisioner/objects/object.go @@ -14,8 +14,6 @@ package objects import ( - "strings" - corev1 "k8s.io/api/core/v1" ) @@ -31,13 +29,3 @@ func NewUnprivilegedPodSecurity() *corev1.PodSecurityContext { RunAsNonRoot: &nonRoot, } } - -// TagFromImage returns the tag from the provided image or an -// empty string if the image does not contain a tag. -func TagFromImage(image string) string { - if strings.Contains(image, ":") { - parsed := strings.Split(image, ":") - return parsed[1] - } - return "" -} diff --git a/internal/provisioner/objects/rbac.go b/internal/provisioner/objects/rbac.go index 092df34ca69..057f1092e70 100644 --- a/internal/provisioner/objects/rbac.go +++ b/internal/provisioner/objects/rbac.go @@ -29,65 +29,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -const ( - // contourRbacNamePrefix is the prefix of the name used for Contour RBAC resources. - contourRbacNamePrefix = "contour" - - // contourRoleBindingNamePrefix is a special case RoleBinding name prefix since - // the certgen RoleBinding name is "contour" - contourRoleBindingNamePrefix = "contour-rolebinding" - - // envoyRbacNamePrefix is the prefix of the name used for Envoy RBAC resources. - envoyRbacNamePrefix = "envoy" - - // certGenRbacNamePrefix is the prefix of the name used for Contour certificate - // generation RBAC resources. - certGenRbacNamePrefix = "contour-certgen" -) - -// RBACNames holds a set of names of related RBAC resources. -type RBACNames struct { - ServiceAccount string - ClusterRole string - ClusterRoleBinding string - Role string - RoleBinding string -} - -// GetContourRBACNames returns the names of the RBAC resources for -// the Contour deployment. -func GetContourRBACNames(contour *model.Contour) RBACNames { - return RBACNames{ - ServiceAccount: fmt.Sprintf("%s-%s", contourRbacNamePrefix, contour.Name), - ClusterRole: fmt.Sprintf("%s-%s-%s", contourRbacNamePrefix, contour.Namespace, contour.Name), - ClusterRoleBinding: fmt.Sprintf("%s-%s-%s", contourRbacNamePrefix, contour.Namespace, contour.Name), - Role: fmt.Sprintf("%s-%s", contourRbacNamePrefix, contour.Name), - - // this one has a different prefix to differentiate from the certgen role binding (see below). - RoleBinding: fmt.Sprintf("%s-%s", contourRoleBindingNamePrefix, contour.Name), - } -} - -// GetEnvoyRBACNames returns the names of the RBAC resources for -// the Envoy daemonset. -func GetEnvoyRBACNames(contour *model.Contour) RBACNames { - return RBACNames{ - ServiceAccount: fmt.Sprintf("%s-%s", envoyRbacNamePrefix, contour.Name), - } -} - -// GetCertgenRBACNames returns the names of the RBAC resources for -// the Certgen job. -func GetCertgenRBACNames(contour *model.Contour) RBACNames { - return RBACNames{ - ServiceAccount: fmt.Sprintf("%s-%s", certGenRbacNamePrefix, contour.Name), - Role: fmt.Sprintf("%s-%s", certGenRbacNamePrefix, contour.Name), - - // this one is name contour- despite being for certgen for legacy reasons. - RoleBinding: fmt.Sprintf("%s-%s", contourRbacNamePrefix, contour.Name), - } -} - // EnsureRBAC ensures all the necessary RBAC resources exist for the // provided contour. func EnsureRBAC(ctx context.Context, cli client.Client, contour *model.Contour) error { @@ -107,7 +48,7 @@ func EnsureRBAC(ctx context.Context, cli client.Client, contour *model.Contour) } func ensureContourRBAC(ctx context.Context, cli client.Client, contour *model.Contour) error { - names := GetContourRBACNames(contour) + names := contour.ContourRBACNames() // Ensure service account. if _, err := objsa.EnsureServiceAccount(ctx, cli, names.ServiceAccount, contour); err != nil { @@ -134,7 +75,7 @@ func ensureContourRBAC(ctx context.Context, cli client.Client, contour *model.Co } func ensureEnvoyRBAC(ctx context.Context, cli client.Client, contour *model.Contour) error { - names := GetEnvoyRBACNames(contour) + names := contour.EnvoyRBACNames() // Ensure service account. if _, err := objsa.EnsureServiceAccount(ctx, cli, names.ServiceAccount, contour); err != nil { @@ -145,7 +86,7 @@ func ensureEnvoyRBAC(ctx context.Context, cli client.Client, contour *model.Cont } func ensureCertgenRBAC(ctx context.Context, cli client.Client, contour *model.Contour) error { - names := GetCertgenRBACNames(contour) + names := contour.CertgenRBACNames() // Ensure service account. if _, err := objsa.EnsureServiceAccount(ctx, cli, names.ServiceAccount, contour); err != nil { @@ -168,10 +109,10 @@ func ensureCertgenRBAC(ctx context.Context, cli client.Client, contour *model.Co func EnsureRBACDeleted(ctx context.Context, cli client.Client, contour *model.Contour) error { var deletions []client.Object - for _, name := range []RBACNames{ - GetContourRBACNames(contour), - GetEnvoyRBACNames(contour), - GetCertgenRBACNames(contour), + for _, name := range []model.RBACNames{ + contour.ContourRBACNames(), + contour.EnvoyRBACNames(), + contour.CertgenRBACNames(), } { if len(name.RoleBinding) > 0 { rolebinding, err := objrb.CurrentRoleBinding(ctx, cli, contour.Namespace, name.RoleBinding) diff --git a/internal/provisioner/objects/service/service.go b/internal/provisioner/objects/service/service.go index 1bd6ff6ad0d..d863769332c 100644 --- a/internal/provisioner/objects/service/service.go +++ b/internal/provisioner/objects/service/service.go @@ -34,10 +34,6 @@ import ( ) const ( - // contourSvcNamePrefix is the prefix of the name of Contour's Service. - contourSvcNamePrefix = "contour" - // envoySvcNamePrefix is the prefix of the name of Envoy's Service. - envoySvcNamePrefix = "envoy" // awsLbBackendProtoAnnotation is a Service annotation that places the AWS ELB into // "TCP" mode so that it does not do HTTP negotiation for HTTPS connections at the // ELB edge. The downside of this is the remote IP address of all connections will @@ -91,16 +87,6 @@ const ( EnvoyNodePortHTTPSPort = int32(30443) ) -// contourServiceName returns the name of Contour's Service resource. -func contourServiceName(contour *model.Contour) string { - return fmt.Sprintf("%s-%s", contourSvcNamePrefix, contour.Name) -} - -// envoyServiceName returns the name of Envoy's service resource. -func envoyServiceName(contour *model.Contour) string { - return fmt.Sprintf("%s-%s", envoySvcNamePrefix, contour.Name) -} - var ( // InternalLBAnnotations maps cloud providers to the provider's annotation // key/value pair used for managing an internal load balancer. For additional @@ -202,7 +188,7 @@ func DesiredContourService(contour *model.Contour) *corev1.Service { svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Namespace: contour.Namespace, - Name: contourServiceName(contour), + Name: contour.ContourServiceName(), Labels: model.OwnerLabels(contour), }, Spec: corev1.ServiceSpec{ @@ -214,7 +200,7 @@ func DesiredContourService(contour *model.Contour) *corev1.Service { TargetPort: intstr.IntOrString{IntVal: xdsPort}, }, }, - Selector: objdeploy.ContourDeploymentPodSelector(contour.Name).MatchLabels, + Selector: objdeploy.ContourDeploymentPodSelector(contour).MatchLabels, Type: corev1.ServiceTypeClusterIP, SessionAffinity: corev1.ServiceAffinityNone, }, @@ -270,13 +256,13 @@ func DesiredEnvoyService(contour *model.Contour) *corev1.Service { svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Namespace: contour.Namespace, - Name: envoyServiceName(contour), + Name: contour.EnvoyServiceName(), Annotations: map[string]string{}, Labels: model.OwnerLabels(contour), }, Spec: corev1.ServiceSpec{ Ports: ports, - Selector: objds.EnvoyDaemonSetPodSelector(contour.Name).MatchLabels, + Selector: objds.EnvoyDaemonSetPodSelector(contour).MatchLabels, SessionAffinity: corev1.ServiceAffinityNone, }, } @@ -362,7 +348,7 @@ func currentContourService(ctx context.Context, cli client.Client, contour *mode current := &corev1.Service{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: contourServiceName(contour), + Name: contour.ContourServiceName(), } err := cli.Get(ctx, key, current) if err != nil { @@ -376,7 +362,7 @@ func currentEnvoyService(ctx context.Context, cli client.Client, contour *model. current := &corev1.Service{} key := types.NamespacedName{ Namespace: contour.Namespace, - Name: envoyServiceName(contour), + Name: contour.EnvoyServiceName(), } err := cli.Get(ctx, key, current) if err != nil { From b06fc00cac85c35c41245b2161c1b3cdfd233810 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Tue, 29 Mar 2022 16:18:22 +0000 Subject: [PATCH 6/6] changelog Signed-off-by: Steve Kriss --- changelogs/unreleased/4426-skriss-minor.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelogs/unreleased/4426-skriss-minor.md diff --git a/changelogs/unreleased/4426-skriss-minor.md b/changelogs/unreleased/4426-skriss-minor.md new file mode 100644 index 00000000000..b1baa6fc21e --- /dev/null +++ b/changelogs/unreleased/4426-skriss-minor.md @@ -0,0 +1,6 @@ +## Gateway provisioner: add support for more than one Gateway/Contour instance per namespace + +The Gateway provisioner now supports having more than one Gateway/Contour instance per namespace. +All resource names now include a `-` suffix to avoid conflicts (cluster-scoped resources also include the namespace as part of the resource name). +Contour instances are always provisioned in the namespace of the Gateway custom resource itself. +