Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the pointer pkg instead of BYO functions everywhere #1423

Merged
merged 1 commit into from
Aug 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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