Skip to content

Commit 561215d

Browse files
hc-github-team-consul-corenathancoleman
hc-github-team-consul-core
and
nathancoleman
authored
Backport of [NET-10985] Fix bug where imagePullSecrets were not set up for Gateways into release/1.5.x (#4374)
* backport of commit 25ba129 * backport of commit 34bbb73 * backport of commit 316985a * backport of commit aa0758f * backport of commit b7c1824 * backport of commit 76415b8 * backport of commit a6b9305 * backport of commit 365b2c2 * backport of commit ac64e83 * backport of commit e6b636d * backport of commit 1e3503c * backport of commit 8260f94 * backport of commit 16fceb9 --------- Co-authored-by: Nathan Coleman <[email protected]>
1 parent 2afc1f3 commit 561215d

File tree

11 files changed

+104
-40
lines changed

11 files changed

+104
-40
lines changed

‎.changelog/4316.txt

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
```release-note:bug
2+
api-gateway: `global.imagePullSecrets` are now configured on the `ServiceAccount` for `Gateways`.
3+
4+
Note: the referenced image pull Secret(s) must be present in the same namespace the `Gateway` is deployed to.
5+
```

‎charts/consul/templates/connect-inject-configmap.yaml

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{{- if .Values.connectInject.enabled }}
2+
apiVersion: v1
3+
kind: ConfigMap
4+
metadata:
5+
name: {{ template "consul.fullname" . }}-connect-inject-config
6+
namespace: {{ .Release.Namespace }}
7+
labels:
8+
app: {{ template "consul.name" . }}
9+
chart: {{ template "consul.chart" . }}
10+
heritage: {{ .Release.Service }}
11+
release: {{ .Release.Name }}
12+
component: connect-injector
13+
data:
14+
config.json: |
15+
{
16+
"image_pull_secrets": {{ .Values.global.imagePullSecrets | toJson }}
17+
}
18+
{{- end }}

‎charts/consul/templates/connect-inject-deployment.yaml

+7
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ spec:
142142
- "-ec"
143143
- |
144144
exec consul-k8s-control-plane inject-connect \
145+
-config-file=/consul/config/config.json \
145146
{{- if .Values.global.federation.enabled }}
146147
-enable-federation \
147148
{{- end }}
@@ -318,6 +319,9 @@ spec:
318319
successThreshold: 1
319320
timeoutSeconds: 5
320321
volumeMounts:
322+
- name: config
323+
mountPath: /consul/config
324+
readOnly: true
321325
{{- if not (and .Values.global.secretsBackend.vault.enabled .Values.global.secretsBackend.vault.connectInject.tlsCert.secretName) }}
322326
- name: certs
323327
mountPath: /etc/connect-injector/certs
@@ -333,6 +337,9 @@ spec:
333337
{{- toYaml . | nindent 12 }}
334338
{{- end }}
335339
volumes:
340+
- name: config
341+
configMap:
342+
name: {{ template "consul.fullname" . }}-connect-inject-config
336343
{{- if not (and .Values.global.secretsBackend.vault.enabled .Values.global.secretsBackend.vault.connectInject.tlsCert.secretName) }}
337344
- name: certs
338345
secret:

‎control-plane/api-gateway/common/helm_config.go

+2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ type HelmConfig struct {
1919
ImageDataplane string
2020
// ImageConsulK8S is the Consul Kubernetes Control Plane image to use in gateway deployments.
2121
ImageConsulK8S string
22+
// ImagePullSecrets reference one or more Secret(s) that contain the credentials to pull images from private image repos.
23+
ImagePullSecrets []v1.LocalObjectReference
2224
// GlobalImagePullPolicy is the pull policy to use for all images used in gateway deployments.
2325
GlobalImagePullPolicy string
2426
ConsulDestinationNamespace string

‎control-plane/api-gateway/gatekeeper/gatekeeper.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,9 @@ func (g *Gatekeeper) namespacedName(gateway gwv1beta1.Gateway) types.NamespacedN
106106
}
107107

108108
func (g *Gatekeeper) serviceAccountName(gateway gwv1beta1.Gateway, config common.HelmConfig) string {
109-
if config.AuthMethod == "" && !config.EnableOpenShift {
109+
// We only create a ServiceAccount if it's needed for RBAC or image pull secrets;
110+
// otherwise, we clean up if one was previously created.
111+
if config.AuthMethod == "" && !config.EnableOpenShift && len(config.ImagePullSecrets) == 0 {
110112
return ""
111113
}
112114
return gateway.Name

‎control-plane/api-gateway/gatekeeper/gatekeeper_test.go

+30-23
Original file line numberDiff line numberDiff line change
@@ -197,12 +197,13 @@ func TestUpsert(t *testing.T) {
197197
},
198198
},
199199
helmConfig: common.HelmConfig{
200-
ImageDataplane: dataplaneImage,
200+
ImageDataplane: dataplaneImage,
201+
ImagePullSecrets: []corev1.LocalObjectReference{{Name: "my-secret"}},
201202
},
202203
initialResources: resources{},
203204
finalResources: resources{
204205
deployments: []*appsv1.Deployment{
205-
configureDeployment(name, namespace, labels, 3, nil, nil, "", "1"),
206+
configureDeployment(name, namespace, labels, 3, nil, nil, name, "1"),
206207
},
207208
roles: []*rbac.Role{},
208209
secrets: []*corev1.Secret{
@@ -224,7 +225,9 @@ func TestUpsert(t *testing.T) {
224225
},
225226
}, "1", false, false),
226227
},
227-
serviceAccounts: []*corev1.ServiceAccount{},
228+
serviceAccounts: []*corev1.ServiceAccount{
229+
configureServiceAccount(name, namespace, labels, "1", []corev1.LocalObjectReference{{Name: "my-secret"}}),
230+
},
228231
},
229232
},
230233
"create a new gateway deployment with managed Service": {
@@ -279,7 +282,6 @@ func TestUpsert(t *testing.T) {
279282
},
280283
}, "1", false, false),
281284
},
282-
serviceAccounts: []*corev1.ServiceAccount{},
283285
},
284286
},
285287
"create a new gateway deployment with managed Service and ACLs": {
@@ -307,13 +309,14 @@ func TestUpsert(t *testing.T) {
307309
},
308310
},
309311
helmConfig: common.HelmConfig{
310-
AuthMethod: "method",
311-
ImageDataplane: dataplaneImage,
312+
AuthMethod: "method",
313+
ImageDataplane: dataplaneImage,
314+
ImagePullSecrets: []corev1.LocalObjectReference{{Name: "my-secret"}},
312315
},
313316
initialResources: resources{},
314317
finalResources: resources{
315318
deployments: []*appsv1.Deployment{
316-
configureDeployment(name, namespace, labels, 3, nil, nil, "", "1"),
319+
configureDeployment(name, namespace, labels, 3, nil, nil, name, "1"),
317320
},
318321
roles: []*rbac.Role{
319322
configureRole(name, namespace, labels, "1", false),
@@ -341,7 +344,7 @@ func TestUpsert(t *testing.T) {
341344
}, "1", false, false),
342345
},
343346
serviceAccounts: []*corev1.ServiceAccount{
344-
configureServiceAccount(name, namespace, labels, "1"),
347+
configureServiceAccount(name, namespace, labels, "1", []corev1.LocalObjectReference{{Name: "my-secret"}}),
345348
},
346349
},
347350
},
@@ -451,7 +454,7 @@ func TestUpsert(t *testing.T) {
451454
},
452455
initialResources: resources{
453456
deployments: []*appsv1.Deployment{
454-
configureDeployment(name, namespace, labels, 3, nil, nil, "", "1"),
457+
configureDeployment(name, namespace, labels, 3, nil, nil, name, "1"),
455458
},
456459
roles: []*rbac.Role{
457460
configureRole(name, namespace, labels, "1", false),
@@ -472,12 +475,12 @@ func TestUpsert(t *testing.T) {
472475
}, "1", true, false),
473476
},
474477
serviceAccounts: []*corev1.ServiceAccount{
475-
configureServiceAccount(name, namespace, labels, "1"),
478+
configureServiceAccount(name, namespace, labels, "1", nil),
476479
},
477480
},
478481
finalResources: resources{
479482
deployments: []*appsv1.Deployment{
480-
configureDeployment(name, namespace, labels, 3, nil, nil, "", "2"),
483+
configureDeployment(name, namespace, labels, 3, nil, nil, name, "2"),
481484
},
482485
roles: []*rbac.Role{
483486
configureRole(name, namespace, labels, "1", false),
@@ -505,7 +508,7 @@ func TestUpsert(t *testing.T) {
505508
}, "2", false, false),
506509
},
507510
serviceAccounts: []*corev1.ServiceAccount{
508-
configureServiceAccount(name, namespace, labels, "1"),
511+
configureServiceAccount(name, namespace, labels, "1", nil),
509512
},
510513
},
511514
ignoreTimestampOnService: true,
@@ -542,7 +545,7 @@ func TestUpsert(t *testing.T) {
542545
},
543546
initialResources: resources{
544547
deployments: []*appsv1.Deployment{
545-
configureDeployment(name, namespace, labels, 3, nil, nil, "", "1"),
548+
configureDeployment(name, namespace, labels, 3, nil, nil, name, "1"),
546549
},
547550
roles: []*rbac.Role{
548551
configureRole(name, namespace, labels, "1", false),
@@ -568,12 +571,12 @@ func TestUpsert(t *testing.T) {
568571
}, "1", true, false),
569572
},
570573
serviceAccounts: []*corev1.ServiceAccount{
571-
configureServiceAccount(name, namespace, labels, "1"),
574+
configureServiceAccount(name, namespace, labels, "1", nil),
572575
},
573576
},
574577
finalResources: resources{
575578
deployments: []*appsv1.Deployment{
576-
configureDeployment(name, namespace, labels, 3, nil, nil, "", "2"),
579+
configureDeployment(name, namespace, labels, 3, nil, nil, name, "2"),
577580
},
578581
roles: []*rbac.Role{
579582
configureRole(name, namespace, labels, "1", false),
@@ -595,7 +598,7 @@ func TestUpsert(t *testing.T) {
595598
}, "2", false, false),
596599
},
597600
serviceAccounts: []*corev1.ServiceAccount{
598-
configureServiceAccount(name, namespace, labels, "1"),
601+
configureServiceAccount(name, namespace, labels, "1", nil),
599602
},
600603
},
601604
ignoreTimestampOnService: true,
@@ -955,7 +958,7 @@ func TestUpsert(t *testing.T) {
955958
},
956959
finalResources: resources{
957960
deployments: []*appsv1.Deployment{
958-
configureDeployment(name, namespace, labels, 3, nil, nil, "", "1"),
961+
configureDeployment(name, namespace, labels, 3, nil, nil, name, "1"),
959962
},
960963
roles: []*rbac.Role{
961964
configureRole(name, namespace, labels, "1", true),
@@ -966,7 +969,7 @@ func TestUpsert(t *testing.T) {
966969
secrets: []*corev1.Secret{},
967970
services: []*corev1.Service{},
968971
serviceAccounts: []*corev1.ServiceAccount{
969-
configureServiceAccount(name, namespace, labels, "1"),
972+
configureServiceAccount(name, namespace, labels, "1", nil),
970973
},
971974
},
972975
},
@@ -1311,7 +1314,7 @@ func TestDelete(t *testing.T) {
13111314
}, "1", true, false),
13121315
},
13131316
serviceAccounts: []*corev1.ServiceAccount{
1314-
configureServiceAccount(name, namespace, labels, "1"),
1317+
configureServiceAccount(name, namespace, labels, "1", nil),
13151318
},
13161319
},
13171320
finalResources: resources{
@@ -1377,7 +1380,7 @@ func TestDelete(t *testing.T) {
13771380
}, "1", true, false),
13781381
},
13791382
serviceAccounts: []*corev1.ServiceAccount{
1380-
configureServiceAccount(name, namespace, labels, "1"),
1383+
configureServiceAccount(name, namespace, labels, "1", nil),
13811384
},
13821385
},
13831386
finalResources: resources{
@@ -1475,6 +1478,9 @@ func validateResourcesExist(t *testing.T, client client.Client, helmConfig commo
14751478
require.Equal(t, expected.Spec.Template.ObjectMeta.Annotations, actual.Spec.Template.ObjectMeta.Annotations)
14761479
require.Equal(t, expected.Spec.Template.ObjectMeta.Labels, actual.Spec.Template.Labels)
14771480

1481+
// Ensure the service account is assigned
1482+
require.Equal(t, expected.Spec.Template.Spec.ServiceAccountName, actual.Spec.Template.Spec.ServiceAccountName)
1483+
14781484
// Ensure there is an init container
14791485
hasInitContainer := false
14801486
for _, container := range actual.Spec.Template.Spec.InitContainers {
@@ -1684,7 +1690,7 @@ func validateResourcesAreDeleted(t *testing.T, k8sClient client.Client, resource
16841690
return nil
16851691
}
16861692

1687-
func configureDeployment(name, namespace string, labels map[string]string, replicas int32, nodeSelector map[string]string, tolerations []corev1.Toleration, serviceAccoutName, resourceVersion string) *appsv1.Deployment {
1693+
func configureDeployment(name, namespace string, labels map[string]string, replicas int32, nodeSelector map[string]string, tolerations []corev1.Toleration, serviceAccountName, resourceVersion string) *appsv1.Deployment {
16881694
return &appsv1.Deployment{
16891695
TypeMeta: metav1.TypeMeta{
16901696
APIVersion: "apps/v1",
@@ -1737,7 +1743,7 @@ func configureDeployment(name, namespace string, labels map[string]string, repli
17371743
},
17381744
NodeSelector: nodeSelector,
17391745
Tolerations: tolerations,
1740-
ServiceAccountName: serviceAccoutName,
1746+
ServiceAccountName: serviceAccountName,
17411747
},
17421748
},
17431749
},
@@ -1886,7 +1892,7 @@ func configureService(name, namespace string, labels, annotations map[string]str
18861892
return &service
18871893
}
18881894

1889-
func configureServiceAccount(name, namespace string, labels map[string]string, resourceVersion string) *corev1.ServiceAccount {
1895+
func configureServiceAccount(name, namespace string, labels map[string]string, resourceVersion string, pullSecrets []corev1.LocalObjectReference) *corev1.ServiceAccount {
18901896
return &corev1.ServiceAccount{
18911897
TypeMeta: metav1.TypeMeta{
18921898
APIVersion: "v1",
@@ -1907,6 +1913,7 @@ func configureServiceAccount(name, namespace string, labels map[string]string, r
19071913
},
19081914
},
19091915
},
1916+
ImagePullSecrets: pullSecrets,
19101917
}
19111918
}
19121919

‎control-plane/api-gateway/gatekeeper/init.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ type initContainerCommandData struct {
3636
LogJSON bool
3737
}
3838

39-
// containerInit returns the init container spec for connect-init that polls for the service and the connect proxy service to be registered
39+
// initContainer returns the init container spec for connect-init that polls for the service and the connect proxy service to be registered
4040
// so that it can save the proxy service id to the shared volume and boostrap Envoy with the proxy-id.
41-
func (g Gatekeeper) initContainer(config common.HelmConfig, name, namespace string) (corev1.Container, error) {
41+
func (g *Gatekeeper) initContainer(config common.HelmConfig, name, namespace string) (corev1.Container, error) {
4242
data := initContainerCommandData{
4343
AuthMethod: config.AuthMethod,
4444
LogLevel: config.LogLevel,

‎control-plane/api-gateway/gatekeeper/rolebinding.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,13 @@ import (
1010
"k8s.io/apimachinery/pkg/types"
1111
gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
1212

13-
"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
14-
"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
1513
rbac "k8s.io/api/rbac/v1"
1614
k8serrors "k8s.io/apimachinery/pkg/api/errors"
1715
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1816
ctrl "sigs.k8s.io/controller-runtime"
17+
18+
"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
19+
"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
1920
)
2021

2122
func (g *Gatekeeper) upsertRoleBinding(ctx context.Context, gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config common.HelmConfig) error {
@@ -65,7 +66,7 @@ func (g *Gatekeeper) deleteRoleBinding(ctx context.Context, gwName types.Namespa
6566

6667
func (g *Gatekeeper) roleBinding(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config common.HelmConfig) *rbac.RoleBinding {
6768
// Create resources for reference. This avoids bugs if naming patterns change.
68-
serviceAccount := g.serviceAccount(gateway)
69+
serviceAccount := g.serviceAccount(gateway, config)
6970
role := g.role(gateway, gcc, config)
7071

7172
return &rbac.RoleBinding{

‎control-plane/api-gateway/gatekeeper/serviceaccount.go

+11-11
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,20 @@ import (
77
"context"
88
"errors"
99

10-
"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
11-
"k8s.io/apimachinery/pkg/types"
12-
gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
13-
1410
corev1 "k8s.io/api/core/v1"
1511
k8serrors "k8s.io/apimachinery/pkg/api/errors"
1612
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/apimachinery/pkg/types"
1714
ctrl "sigs.k8s.io/controller-runtime"
15+
gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
16+
17+
"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
1818
)
1919

2020
func (g *Gatekeeper) upsertServiceAccount(ctx context.Context, gateway gwv1beta1.Gateway, config common.HelmConfig) error {
21-
if config.AuthMethod == "" && !config.EnableOpenShift {
21+
// We only create a ServiceAccount if it's needed for RBAC or image pull secrets;
22+
// otherwise, we clean up if one was previously created.
23+
if config.AuthMethod == "" && !config.EnableOpenShift && len(config.ImagePullSecrets) == 0 {
2224
return g.deleteServiceAccount(ctx, types.NamespacedName{Namespace: gateway.Namespace, Name: gateway.Name})
2325
}
2426

@@ -47,15 +49,12 @@ func (g *Gatekeeper) upsertServiceAccount(ctx context.Context, gateway gwv1beta1
4749
}
4850

4951
// Create the ServiceAccount.
50-
serviceAccount = g.serviceAccount(gateway)
52+
serviceAccount = g.serviceAccount(gateway, config)
5153
if err := ctrl.SetControllerReference(&gateway, serviceAccount, g.Client.Scheme()); err != nil {
5254
return err
5355
}
54-
if err := g.Client.Create(ctx, serviceAccount); err != nil {
55-
return err
56-
}
5756

58-
return nil
57+
return g.Client.Create(ctx, serviceAccount)
5958
}
6059

6160
func (g *Gatekeeper) deleteServiceAccount(ctx context.Context, gwName types.NamespacedName) error {
@@ -69,12 +68,13 @@ func (g *Gatekeeper) deleteServiceAccount(ctx context.Context, gwName types.Name
6968
return nil
7069
}
7170

72-
func (g *Gatekeeper) serviceAccount(gateway gwv1beta1.Gateway) *corev1.ServiceAccount {
71+
func (g *Gatekeeper) serviceAccount(gateway gwv1beta1.Gateway, config common.HelmConfig) *corev1.ServiceAccount {
7372
return &corev1.ServiceAccount{
7473
ObjectMeta: metav1.ObjectMeta{
7574
Name: gateway.Name,
7675
Namespace: gateway.Namespace,
7776
Labels: common.LabelsForGateway(&gateway),
7877
},
78+
ImagePullSecrets: config.ImagePullSecrets,
7979
}
8080
}

‎control-plane/subcommand/inject-connect/command.go

+2
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ type Command struct {
5151
flagListen string
5252
flagCertDir string // Directory with TLS certs for listening (PEM)
5353
flagDefaultInject bool // True to inject by default
54+
flagConfigFile string // Path to a config file in JSON format
5455
flagConsulImage string // Docker image for Consul
5556
flagConsulDataplaneImage string // Docker image for Envoy
5657
flagConsulK8sImage string // Docker image for consul-k8s
@@ -184,6 +185,7 @@ func init() {
184185
func (c *Command) init() {
185186
c.flagSet = flag.NewFlagSet("", flag.ContinueOnError)
186187
c.flagSet.StringVar(&c.flagListen, "listen", ":8080", "Address to bind listener to.")
188+
c.flagSet.StringVar(&c.flagConfigFile, "config-file", "", "Path to a JSON config file.")
187189
c.flagSet.Var((*flags.FlagMapValue)(&c.flagNodeMeta), "node-meta",
188190
"Metadata to set on the node, formatted as key=value. This flag may be specified multiple times to set multiple meta fields.")
189191
c.flagSet.BoolVar(&c.flagDefaultInject, "default-inject", true, "Inject by default.")

0 commit comments

Comments
 (0)