Skip to content

Commit

Permalink
Use the pointer pkg instead of BYO functions everywhere (#1423)
Browse files Browse the repository at this point in the history
  • Loading branch information
kschoche authored Aug 15, 2022
1 parent e5d1770 commit e51cb6d
Show file tree
Hide file tree
Showing 10 changed files with 60 additions and 65 deletions.
32 changes: 9 additions & 23 deletions control-plane/connect-inject/container_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

corev1 "k8s.io/api/core/v1"
"k8s.io/utils/pointer"
)

const (
Expand Down Expand Up @@ -117,10 +118,10 @@ func (w *MeshWebhook) initCopyContainer() corev1.Container {
if !w.EnableOpenShift {
container.SecurityContext = &corev1.SecurityContext{
// Set RunAsUser because the default user for the consul container is root and we want to run non-root.
RunAsUser: pointerToInt64(copyContainerUserAndGroupID),
RunAsGroup: pointerToInt64(copyContainerUserAndGroupID),
RunAsNonRoot: pointerToBool(true),
ReadOnlyRootFilesystem: pointerToBool(true),
RunAsUser: pointer.Int64(copyContainerUserAndGroupID),
RunAsGroup: pointer.Int64(copyContainerUserAndGroupID),
RunAsNonRoot: pointer.Bool(true),
ReadOnlyRootFilesystem: pointer.Bool(true),
}
}
return container
Expand Down Expand Up @@ -297,11 +298,11 @@ func (w *MeshWebhook) containerInit(namespace corev1.Namespace, pod corev1.Pod,
// Running consul connect redirect-traffic with iptables
// requires both being a root user and having NET_ADMIN capability.
container.SecurityContext = &corev1.SecurityContext{
RunAsUser: pointerToInt64(rootUserAndGroupID),
RunAsGroup: pointerToInt64(rootUserAndGroupID),
RunAsUser: pointer.Int64(rootUserAndGroupID),
RunAsGroup: pointer.Int64(rootUserAndGroupID),
// RunAsNonRoot overrides any setting in the Pod so that we can still run as root here as required.
RunAsNonRoot: pointerToBool(false),
Privileged: pointerToBool(true),
RunAsNonRoot: pointer.Bool(false),
Privileged: pointer.Bool(true),
Capabilities: &corev1.Capabilities{
Add: []corev1.Capability{netAdminCapability},
},
Expand Down Expand Up @@ -352,21 +353,6 @@ func consulDNSEnabled(namespace corev1.Namespace, pod corev1.Pod, globalEnabled
return globalEnabled, nil
}

// pointerToInt64 takes an int64 and returns a pointer to it.
func pointerToInt64(i int64) *int64 {
return &i
}

// pointerToUInt64 takes an int64 and returns a pointer to it.
func pointerToUint64(i uint64) *uint64 {
return &i
}

// pointerToBool takes a bool and returns a pointer to it.
func pointerToBool(b bool) *bool {
return &b
}

// splitCommaSeparatedItemsFromAnnotation takes an annotation and a pod
// and returns the comma-separated value of the annotation as a list of strings.
func splitCommaSeparatedItemsFromAnnotation(annotation string, pod corev1.Pod) []string {
Expand Down
17 changes: 9 additions & 8 deletions control-plane/connect-inject/container_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"
)

const k8sNamespace = "k8snamespace"
Expand Down Expand Up @@ -371,13 +372,13 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
pod.Annotations = c.annotations

expectedSecurityContext := &corev1.SecurityContext{
RunAsUser: pointerToInt64(0),
RunAsGroup: pointerToInt64(0),
Privileged: pointerToBool(true),
RunAsUser: pointer.Int64(0),
RunAsGroup: pointer.Int64(0),
Privileged: pointer.Bool(true),
Capabilities: &corev1.Capabilities{
Add: []corev1.Capability{netAdminCapability},
},
RunAsNonRoot: pointerToBool(false),
RunAsNonRoot: pointer.Bool(false),
}
ns := testNS
ns.Labels = c.namespaceLabel
Expand Down Expand Up @@ -1166,10 +1167,10 @@ func TestHandlerInitCopyContainer(t *testing.T) {
require.Nil(t, container.SecurityContext)
} else {
expectedSecurityContext := &corev1.SecurityContext{
RunAsUser: pointerToInt64(copyContainerUserAndGroupID),
RunAsGroup: pointerToInt64(copyContainerUserAndGroupID),
RunAsNonRoot: pointerToBool(true),
ReadOnlyRootFilesystem: pointerToBool(true),
RunAsUser: pointer.Int64(copyContainerUserAndGroupID),
RunAsGroup: pointer.Int64(copyContainerUserAndGroupID),
RunAsNonRoot: pointer.Bool(true),
ReadOnlyRootFilesystem: pointer.Bool(true),
}
require.Equal(t, expectedSecurityContext, container.SecurityContext)
}
Expand Down
9 changes: 5 additions & 4 deletions control-plane/connect-inject/envoy_sidecar.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/google/shlex"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/utils/pointer"
)

func (w *MeshWebhook) envoySidecar(namespace corev1.Namespace, pod corev1.Pod, mpi multiPortInfo) (corev1.Container, error) {
Expand Down Expand Up @@ -84,10 +85,10 @@ func (w *MeshWebhook) envoySidecar(namespace corev1.Namespace, pod corev1.Pod, m
}
}
container.SecurityContext = &corev1.SecurityContext{
RunAsUser: pointerToInt64(envoyUserAndGroupID),
RunAsGroup: pointerToInt64(envoyUserAndGroupID),
RunAsNonRoot: pointerToBool(true),
ReadOnlyRootFilesystem: pointerToBool(true),
RunAsUser: pointer.Int64(envoyUserAndGroupID),
RunAsGroup: pointer.Int64(envoyUserAndGroupID),
RunAsNonRoot: pointer.Bool(true),
ReadOnlyRootFilesystem: pointer.Bool(true),
}
}

Expand Down
35 changes: 18 additions & 17 deletions control-plane/connect-inject/envoy_sidecar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"
)

func TestHandlerEnvoySidecar(t *testing.T) {
Expand Down Expand Up @@ -137,20 +138,20 @@ func TestHandlerEnvoySidecar_withSecurityContext(t *testing.T) {
tproxyEnabled: false,
openShiftEnabled: false,
expSecurityContext: &corev1.SecurityContext{
RunAsUser: pointerToInt64(envoyUserAndGroupID),
RunAsGroup: pointerToInt64(envoyUserAndGroupID),
RunAsNonRoot: pointerToBool(true),
ReadOnlyRootFilesystem: pointerToBool(true),
RunAsUser: pointer.Int64(envoyUserAndGroupID),
RunAsGroup: pointer.Int64(envoyUserAndGroupID),
RunAsNonRoot: pointer.Bool(true),
ReadOnlyRootFilesystem: pointer.Bool(true),
},
},
"tproxy enabled; openshift disabled": {
tproxyEnabled: true,
openShiftEnabled: false,
expSecurityContext: &corev1.SecurityContext{
RunAsUser: pointerToInt64(envoyUserAndGroupID),
RunAsGroup: pointerToInt64(envoyUserAndGroupID),
RunAsNonRoot: pointerToBool(true),
ReadOnlyRootFilesystem: pointerToBool(true),
RunAsUser: pointer.Int64(envoyUserAndGroupID),
RunAsGroup: pointer.Int64(envoyUserAndGroupID),
RunAsNonRoot: pointer.Bool(true),
ReadOnlyRootFilesystem: pointer.Bool(true),
},
},
"tproxy disabled; openshift enabled": {
Expand All @@ -162,10 +163,10 @@ func TestHandlerEnvoySidecar_withSecurityContext(t *testing.T) {
tproxyEnabled: true,
openShiftEnabled: true,
expSecurityContext: &corev1.SecurityContext{
RunAsUser: pointerToInt64(envoyUserAndGroupID),
RunAsGroup: pointerToInt64(envoyUserAndGroupID),
RunAsNonRoot: pointerToBool(true),
ReadOnlyRootFilesystem: pointerToBool(true),
RunAsUser: pointer.Int64(envoyUserAndGroupID),
RunAsGroup: pointer.Int64(envoyUserAndGroupID),
RunAsNonRoot: pointer.Bool(true),
ReadOnlyRootFilesystem: pointer.Bool(true),
},
},
}
Expand Down Expand Up @@ -210,7 +211,7 @@ func TestHandlerEnvoySidecar_FailsWithDuplicatePodSecurityContextUID(t *testing.
},
},
SecurityContext: &corev1.PodSecurityContext{
RunAsUser: pointerToInt64(envoyUserAndGroupID),
RunAsUser: pointer.Int64(envoyUserAndGroupID),
},
},
}
Expand Down Expand Up @@ -238,14 +239,14 @@ func TestHandlerEnvoySidecar_FailsWithDuplicateContainerSecurityContextUID(t *te
Name: "web",
// Setting RunAsUser: 1 should succeed.
SecurityContext: &corev1.SecurityContext{
RunAsUser: pointerToInt64(1),
RunAsUser: pointer.Int64(1),
},
},
{
Name: "app",
// Setting RunAsUser: 5995 should fail.
SecurityContext: &corev1.SecurityContext{
RunAsUser: pointerToInt64(envoyUserAndGroupID),
RunAsUser: pointer.Int64(envoyUserAndGroupID),
},
Image: "not-envoy",
},
Expand All @@ -265,14 +266,14 @@ func TestHandlerEnvoySidecar_FailsWithDuplicateContainerSecurityContextUID(t *te
Name: "web",
// Setting RunAsUser: 1 should succeed.
SecurityContext: &corev1.SecurityContext{
RunAsUser: pointerToInt64(1),
RunAsUser: pointer.Int64(1),
},
},
{
Name: "sidecar",
// Setting RunAsUser: 5995 should succeed if the image matches h.ImageEnvoy.
SecurityContext: &corev1.SecurityContext{
RunAsUser: pointerToInt64(envoyUserAndGroupID),
RunAsUser: pointer.Int64(envoyUserAndGroupID),
},
Image: "envoy",
},
Expand Down
3 changes: 2 additions & 1 deletion control-plane/connect-inject/peering_acceptor_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/pointer"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -257,7 +258,7 @@ func (r *PeeringAcceptorController) updateStatus(ctx context.Context, acceptorOb
return err
}
if acceptor.Status.LatestPeeringVersion == nil || *acceptor.Status.LatestPeeringVersion < peeringVersion {
acceptor.Status.LatestPeeringVersion = pointerToUint64(peeringVersion)
acceptor.Status.LatestPeeringVersion = pointer.Uint64(peeringVersion)
}
}
err := r.Status().Update(ctx, acceptor)
Expand Down
11 changes: 6 additions & 5 deletions control-plane/connect-inject/peering_acceptor_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/utils/pointer"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand Down Expand Up @@ -294,7 +295,7 @@ func TestReconcile_CreateUpdatePeeringAcceptor(t *testing.T) {
Backend: "kubernetes",
},
},
LatestPeeringVersion: pointerToUint64(2),
LatestPeeringVersion: pointer.Uint64(2),
},
expectedConsulPeerings: []*api.Peering{
{
Expand Down Expand Up @@ -774,7 +775,7 @@ func TestReconcile_VersionAnnotation(t *testing.T) {
},
ResourceVersion: "some-old-sha",
},
LatestPeeringVersion: pointerToUint64(3),
LatestPeeringVersion: pointer.Uint64(3),
},
},
"is no/op if annotation value is equal to value in status": {
Expand All @@ -790,7 +791,7 @@ func TestReconcile_VersionAnnotation(t *testing.T) {
},
ResourceVersion: "some-old-sha",
},
LatestPeeringVersion: pointerToUint64(3),
LatestPeeringVersion: pointer.Uint64(3),
},
},
"updates if annotation value is greater than value in status": {
Expand All @@ -805,7 +806,7 @@ func TestReconcile_VersionAnnotation(t *testing.T) {
Backend: "kubernetes",
},
},
LatestPeeringVersion: pointerToUint64(4),
LatestPeeringVersion: pointer.Uint64(4),
},
},
}
Expand Down Expand Up @@ -836,7 +837,7 @@ func TestReconcile_VersionAnnotation(t *testing.T) {
},
ResourceVersion: "some-old-sha",
},
LatestPeeringVersion: pointerToUint64(3),
LatestPeeringVersion: pointer.Uint64(3),
},
}
secret := createSecret("acceptor-created-secret", "default", "data", "some-data")
Expand Down
3 changes: 2 additions & 1 deletion control-plane/connect-inject/peering_dialer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/pointer"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -213,7 +214,7 @@ func (r *PeeringDialerController) updateStatus(ctx context.Context, dialerObjKey
return err
}
if dialer.Status.LatestPeeringVersion == nil || *dialer.Status.LatestPeeringVersion < peeringVersion {
dialer.Status.LatestPeeringVersion = pointerToUint64(peeringVersion)
dialer.Status.LatestPeeringVersion = pointer.Uint64(peeringVersion)
}
}
err := r.Status().Update(ctx, dialer)
Expand Down
11 changes: 6 additions & 5 deletions control-plane/connect-inject/peering_dialer_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/utils/pointer"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand Down Expand Up @@ -239,7 +240,7 @@ func TestReconcile_CreateUpdatePeeringDialer(t *testing.T) {
Backend: "kubernetes",
},
},
LatestPeeringVersion: pointerToUint64(2),
LatestPeeringVersion: pointer.Uint64(2),
},
peeringExists: true,
},
Expand Down Expand Up @@ -390,7 +391,7 @@ func TestReconcile_VersionAnnotationPeeringDialer(t *testing.T) {
Backend: "kubernetes",
},
},
LatestPeeringVersion: pointerToUint64(3),
LatestPeeringVersion: pointer.Uint64(3),
},
},
"is no/op if annotation value is equal to value in status": {
Expand All @@ -405,7 +406,7 @@ func TestReconcile_VersionAnnotationPeeringDialer(t *testing.T) {
Backend: "kubernetes",
},
},
LatestPeeringVersion: pointerToUint64(3),
LatestPeeringVersion: pointer.Uint64(3),
},
},
"updates if annotation value is greater than value in status": {
Expand All @@ -420,7 +421,7 @@ func TestReconcile_VersionAnnotationPeeringDialer(t *testing.T) {
Backend: "kubernetes",
},
},
LatestPeeringVersion: pointerToUint64(4),
LatestPeeringVersion: pointer.Uint64(4),
},
},
}
Expand Down Expand Up @@ -466,7 +467,7 @@ func TestReconcile_VersionAnnotationPeeringDialer(t *testing.T) {
},
ResourceVersion: "latest-version",
},
LatestPeeringVersion: pointerToUint64(3),
LatestPeeringVersion: pointer.Uint64(3),
},
}
// Create fake k8s client
Expand Down
2 changes: 1 addition & 1 deletion control-plane/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ require (
k8s.io/apiextensions-apiserver v0.22.2 // indirect
k8s.io/component-base v0.22.2 // indirect
k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e // indirect
k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a // indirect
k8s.io/utils v0.0.0-20220812165043-ad590609e2e5 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.1.2 // indirect
sigs.k8s.io/yaml v1.2.0 // indirect
)
Expand Down
2 changes: 2 additions & 0 deletions control-plane/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,8 @@ k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e/go.mod h1:vHXdDvt9+2spS2R
k8s.io/utils v0.0.0-20200324210504-a9aa75ae1b89/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew=
k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a h1:8dYfu/Fc9Gz2rNJKB9IQRGgQOh2clmRzNIPPY1xLY5g=
k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
k8s.io/utils v0.0.0-20220812165043-ad590609e2e5 h1:XmRqFcQlCy/lKRZ39j+RVpokYNroHPqV3mcBRfnhT5o=
k8s.io/utils v0.0.0-20220812165043-ad590609e2e5/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=
rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
Expand Down

0 comments on commit e51cb6d

Please sign in to comment.