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

Automatically adjust Elastic Agent hostPath permissions #6599

Closed
wants to merge 11 commits into from
16 changes: 11 additions & 5 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,13 @@ func startOperator(ctx context.Context) error {
return err
}

setDefaultSecurityContext, err := determineSetDefaultSecurityContext(viper.GetString(operator.SetDefaultSecurityContextFlag), clientset)
openshift, err := isOpenShift(clientset)
if err != nil {
log.Error(err, "failed to determine whether the operator is running within openshift")
return err
}

setDefaultSecurityContext, err := determineSetDefaultSecurityContext(viper.GetString(operator.SetDefaultSecurityContextFlag), openshift)
if err != nil {
log.Error(err, "failed to determine how to set default security context")
return err
Expand Down Expand Up @@ -654,6 +660,7 @@ func startOperator(ctx context.Context) error {
SetDefaultSecurityContext: setDefaultSecurityContext,
ValidateStorageClass: viper.GetBool(operator.ValidateStorageClassFlag),
Tracer: tracer,
IsOpenshift: openshift,
}

if viper.GetBool(operator.EnableWebhookFlag) {
Expand Down Expand Up @@ -788,10 +795,9 @@ func chooseAndValidateIPFamily(ipFamilyStr string, ipFamilyDefault corev1.IPFami
// 2. use OpenShift detection to determine whether or not we are running within an OpenShift cluster.
// If we determine we are on an OpenShift cluster, and since OpenShift automatically sets security context, return false,
// otherwise, return true as we'll need to set this security context on non-OpenShift clusters.
func determineSetDefaultSecurityContext(setDefaultSecurityContext string, clientset kubernetes.Interface) (bool, error) {
func determineSetDefaultSecurityContext(setDefaultSecurityContext string, openshift bool) (bool, error) {
if setDefaultSecurityContext == "auto-detect" {
openshift, err := isOpenShift(clientset)
return !openshift, err
return !openshift, nil
}
return strconv.ParseBool(setDefaultSecurityContext)
}
Expand Down Expand Up @@ -827,7 +833,7 @@ func isOpenShift(clientset kubernetes.Interface) (bool, error) {
}

// We could not determine that we are running on an OpenShift cluster,
// so we will behave as if "setDefaultSecurityContext" was set to true.
// so return false.
return false, nil
}

Expand Down
188 changes: 109 additions & 79 deletions cmd/manager/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,104 +197,63 @@ func Test_garbageCollectSoftOwnedSecrets(t *testing.T) {
}
}

func Test_determineSetDefaultSecurityContext(t *testing.T) {
type args struct {
setDefaultSecurityContext string
clientset kubernetes.Interface
}
func Test_isOpenShift(t *testing.T) {
tests := []struct {
name string
args args
want bool
wantErr bool
name string
clientset kubernetes.Interface
want bool
wantErr bool
}{
{
"auto-detect on OpenShift cluster does not set security context",
args{
"auto-detect",
newFakeK8sClientsetWithDiscovery([]*metav1.APIResourceList{
{
GroupVersion: schema.GroupVersion{Group: "security.openshift.io", Version: "v1"}.String(),
APIResources: []metav1.APIResource{
{
Name: "securitycontextconstraints",
},
name: "on OpenShift cluster is detected properly",
clientset: newFakeK8sClientsetWithDiscovery([]*metav1.APIResourceList{
{
GroupVersion: schema.GroupVersion{Group: "security.openshift.io", Version: "v1"}.String(),
APIResources: []metav1.APIResource{
{
Name: "securitycontextconstraints",
},
},
}, nil),
},
false,
false,
},
}, nil),
want: true,
wantErr: false,
},
{
"auto-detect on OpenShift cluster, returning group discovery failed error for OpenShift security group+version, does not set security context",
args{
"auto-detect",
newFakeK8sClientsetWithDiscovery([]*metav1.APIResourceList{}, &discovery.ErrGroupDiscoveryFailed{
Groups: map[schema.GroupVersion]error{
{Group: "security.openshift.io", Version: "v1"}: nil,
},
}),
},
false,
false,
name: "on OpenShift cluster, returning group discovery failed error for OpenShift security group+version returns true",
clientset: newFakeK8sClientsetWithDiscovery([]*metav1.APIResourceList{}, &discovery.ErrGroupDiscoveryFailed{
Groups: map[schema.GroupVersion]error{
{Group: "security.openshift.io", Version: "v1"}: nil,
},
}),
want: true,
wantErr: false,
},
{
"auto-detect on non-OpenShift cluster, returning not found error, sets security context",
args{
"auto-detect",
newFakeK8sClientsetWithDiscovery([]*metav1.APIResourceList{}, apierrors.NewNotFound(schema.GroupResource{
Group: "security.openshift.io",
Resource: "none",
}, "fake")),
},
true,
false,
name: "not on non-OpenShift cluster, returning not found error returns false",
clientset: newFakeK8sClientsetWithDiscovery([]*metav1.APIResourceList{}, apierrors.NewNotFound(schema.GroupResource{
Group: "security.openshift.io",
Resource: "none",
}, "fake")),
want: false,
wantErr: false,
},
{
"auto-detect on non-OpenShift cluster, returning random error, returns error",
args{
"auto-detect",
newFakeK8sClientsetWithDiscovery([]*metav1.APIResourceList{}, fmt.Errorf("random error")),
},
true,
true,
},
{
"true set, returning no error, will set security context",
args{
"true",
newFakeK8sClientsetWithDiscovery([]*metav1.APIResourceList{}, nil),
},
true,
false,
}, {
"false set, returning no error, will not set security context",
args{
"false",
newFakeK8sClientsetWithDiscovery([]*metav1.APIResourceList{}, nil),
},
false,
false,
}, {
"invalid bool set, returns error",
args{
"invalid",
newFakeK8sClientsetWithDiscovery([]*metav1.APIResourceList{}, nil),
},
false,
true,
name: "random error defaults to false and returns error",
clientset: newFakeK8sClientsetWithDiscovery([]*metav1.APIResourceList{}, fmt.Errorf("random error")),
want: false,
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := determineSetDefaultSecurityContext(tt.args.setDefaultSecurityContext, tt.args.clientset)
got, err := isOpenShift(tt.clientset)
if (err != nil) != tt.wantErr {
t.Errorf("determineSetDefaultSecurityContext() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("isOpenShift() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("determineSetDefaultSecurityContext() = %v, want %v", got, tt.want)
t.Errorf("isOpenShift() = %v, want %v", got, tt.want)
}
})
}
Expand Down Expand Up @@ -337,3 +296,74 @@ func newFakeK8sClientsetWithDiscovery(resources []*metav1.APIResourceList, disco
}
return client
}

func Test_determineSetDefaultSecurityContext(t *testing.T) {
type args struct {
setDefaultSecurityContext string
openshift bool
}
tests := []struct {
name string
args args
want bool
wantErr bool
}{
{
name: "on openshift with auto-detect set return false",
args: args{
setDefaultSecurityContext: "auto-detect",
openshift: true,
},
want: false,
wantErr: false,
},
{
name: "not on openshift with auto-detect set return true",
args: args{
setDefaultSecurityContext: "auto-detect",
openshift: false,
},
want: true,
wantErr: false,
},
{
name: "not on openshift with set-default-security-context set to true returns true",
args: args{
setDefaultSecurityContext: "true",
openshift: false,
},
want: true,
wantErr: false,
},
{
name: "on openshift with set-default-security-context set to true returns true",
args: args{
setDefaultSecurityContext: "true",
openshift: false,
},
want: true,
wantErr: false,
},
{
name: "invalid bool for set-default-security-context returns false and error",
args: args{
setDefaultSecurityContext: "invalid",
openshift: false,
},
want: false,
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := determineSetDefaultSecurityContext(tt.args.setDefaultSecurityContext, tt.args.openshift)
if (err != nil) != tt.wantErr {
t.Errorf("determineSetDefaultSecurityContext() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("determineSetDefaultSecurityContext() = %v, want %v", got, tt.want)
}
})
}
}
6 changes: 0 additions & 6 deletions config/recipes/elastic-agent/fleet-apm-integration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ spec:
spec:
serviceAccountName: fleet-server
automountServiceAccountToken: true
securityContext:
runAsUser: 0
---
apiVersion: agent.k8s.elastic.co/v1alpha1
kind: Agent
Expand All @@ -102,10 +100,6 @@ spec:
mode: fleet
deployment:
replicas: 1
podTemplate:
spec:
securityContext:
runAsUser: 0
---
apiVersion: v1
kind: Service
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,6 @@ spec:
spec:
serviceAccountName: fleet-server
automountServiceAccountToken: true
securityContext:
runAsUser: 0
---
apiVersion: agent.k8s.elastic.co/v1alpha1
kind: Agent
Expand All @@ -114,8 +112,6 @@ spec:
spec:
serviceAccountName: elastic-agent
automountServiceAccountToken: true
securityContext:
runAsUser: 0
containers:
- name: agent
volumeMounts:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,6 @@ spec:
spec:
serviceAccountName: fleet-server
automountServiceAccountToken: true
securityContext:
runAsUser: 0
Copy link
Contributor

@barkbay barkbay Apr 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is still required for the container's CA bundle to be updated, see:

---
apiVersion: agent.k8s.elastic.co/v1alpha1
kind: Agent
Expand All @@ -97,11 +95,7 @@ spec:
podTemplate:
spec:
serviceAccountName: elastic-agent
hostNetwork: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we removing hostNetwork: true?

dnsPolicy: ClusterFirstWithHostNet
automountServiceAccountToken: true
securityContext:
runAsUser: 0
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
Expand Down
2 changes: 0 additions & 2 deletions config/recipes/elastic-agent/kubernetes-integration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ spec:
serviceAccountName: elastic-agent
containers:
- name: agent
securityContext:
runAsUser: 0
env:
- name: NODE_NAME
valueFrom:
Expand Down
2 changes: 0 additions & 2 deletions config/recipes/elastic-agent/multi-output.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ spec:
spec:
containers:
- name: agent
securityContext:
runAsUser: 0
volumeMounts:
- mountPath: /var/log
name: varlog
Expand Down
2 changes: 0 additions & 2 deletions config/recipes/elastic-agent/system-integration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ spec:
spec:
containers:
- name: agent
securityContext:
runAsUser: 0
config:
id: 488e0b80-3634-11eb-8208-57893829af4e
revision: 2
Expand Down
Loading