Skip to content

fix(security): skip OpenShift ETCD monitoring when CR namespace differs from operator namespace #553

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.

Sign up for GitHub

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

Draft
cvrkota wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(security): skip OpenShift ETCD monitoring when CR namespace differs from operator namespace #553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 19 additions & 0 deletions controllers/apply.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package controllers
import (
"context"
"fmt"
"os"
"sort"
"strings"

Expand Down Expand Up @@ -478,6 +479,24 @@ func CreateDeploymentContext(
discoverETCD ETCDDiscoverFunc,
) (*k8ssensordeployment.DeploymentContext, error) {
if isOpenShift {
operatorNamespace := os.Getenv("POD_NAMESPACE")
if operatorNamespace == "" {
logger.Info(
"Skipping OpenShift ETCD monitoring: POD_NAMESPACE environment variable is not set",
)
return nil, nil
}
if agent.Namespace != operatorNamespace {
logger.Info(
"Skipping OpenShift ETCD monitoring: agent CR namespace differs from operator namespace. "+
"ETCD credentials are only copied when the agent runs in the operator's own namespace.",
"agentNamespace",
agent.Namespace,
"operatorNamespace",
operatorNamespace,
)
return nil, nil
}
return setupOpenShiftETCDMonitoring(ctx, c, agent, logger), nil
}

Expand Down
147 changes: 106 additions & 41 deletions controllers/apply_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package controllers
import (
"context"
"errors"
"os"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -74,45 +75,109 @@ func TestCreateDeploymentContext_SimplifiedTests(t *testing.T) {
ctx := context.Background()
logger := zap.New()

t.Run("OpenShift discovers and copies ETCD resources", func(t *testing.T) {
mockClient := &mocks.MockInstanaAgentClient{}

// Mock the Apply method for copying ETCD ConfigMap and Secret
mockClient.On("Apply", mock.Anything, mock.AnythingOfType("*v1.ConfigMap"), mock.Anything).
Return(result.OfSuccess[client.Object](nil))
mockClient.On("Apply", mock.Anything, mock.AnythingOfType("*v1.Secret"), mock.Anything).
Return(result.OfSuccess[client.Object](nil))

// Mock Get calls for ETCD resource checks with valid data
mockClient.On(
"Get",
mock.Anything,
mock.Anything,
mock.AnythingOfType("*v1.ConfigMap"),
mock.Anything,
).Run(func(args mock.Arguments) {
cm := args.Get(2).(*corev1.ConfigMap)
cm.Data = map[string]string{
"ca-bundle.crt": "test-ca-cert-data",
t.Run(
"OpenShift discovers and copies ETCD resources when namespaces match",
func(t *testing.T) {
require.NoError(t, os.Setenv("POD_NAMESPACE", agent.Namespace))
defer func() { require.NoError(t, os.Unsetenv("POD_NAMESPACE")) }()

mockClient := &mocks.MockInstanaAgentClient{}

// Mock the Apply method for copying ETCD ConfigMap and Secret
mockClient.On("Apply", mock.Anything, mock.AnythingOfType("*v1.ConfigMap"), mock.Anything).
Return(result.OfSuccess[client.Object](nil))
mockClient.On("Apply", mock.Anything, mock.AnythingOfType("*v1.Secret"), mock.Anything).
Return(result.OfSuccess[client.Object](nil))

// Mock Get calls for ETCD resource checks with valid data
mockClient.On(
"Get",
mock.Anything,
mock.Anything,
mock.AnythingOfType("*v1.ConfigMap"),
mock.Anything,
).Run(func(args mock.Arguments) {
cm := args.Get(2).(*corev1.ConfigMap)
cm.Data = map[string]string{
"ca-bundle.crt": "test-ca-cert-data",
}
cm.ResourceVersion = "12345"
}).Return(nil)
mockClient.On(
"Get",
mock.Anything,
mock.Anything,
mock.AnythingOfType("*v1.Secret"),
mock.Anything,
).Run(func(args mock.Arguments) {
secret := args.Get(2).(*corev1.Secret)
secret.Data = map[string][]byte{
"tls.crt": []byte("test-cert-data"),
"tls.key": []byte("test-key-data"),
}
secret.ResourceVersion = "67890"
}).Return(nil)

// Mock ETCD discover function (won't be called for OpenShift)
mockDiscoverETCD := func(ctx context.Context, agent *instanav1.InstanaAgent) (*DiscoveredETCDTargets, error) {
return nil, nil
}
cm.ResourceVersion = "12345"
}).Return(nil)
mockClient.On(
"Get",
mock.Anything,
mock.Anything,
mock.AnythingOfType("*v1.Secret"),
mock.Anything,
).Run(func(args mock.Arguments) {
secret := args.Get(2).(*corev1.Secret)
secret.Data = map[string][]byte{
"tls.crt": []byte("test-cert-data"),
"tls.key": []byte("test-key-data"),

deploymentContext, err := CreateDeploymentContext(
ctx,
mockClient,
agent,
true,
logger,
mockDiscoverETCD,
)

require.NoError(t, err)
require.NotNil(t, deploymentContext)
assert.True(
t,
deploymentContext.OpenShiftETCDResourcesExist,
"ETCD resources should exist when Get calls succeed",
)
mockClient.AssertExpectations(t)
},
)

t.Run(
"OpenShift skips ETCD monitoring when agent namespace differs from operator namespace",
func(t *testing.T) {
require.NoError(t, os.Setenv("POD_NAMESPACE", "instana-agent"))
defer func() { require.NoError(t, os.Unsetenv("POD_NAMESPACE")) }()

mockClient := &mocks.MockInstanaAgentClient{}
mockDiscoverETCD := func(ctx context.Context, agent *instanav1.InstanaAgent) (*DiscoveredETCDTargets, error) {
return nil, nil
}
secret.ResourceVersion = "67890"
}).Return(nil)

// Mock ETCD discover function (won't be called for OpenShift)
deploymentContext, err := CreateDeploymentContext(
ctx,
mockClient,
agent, // agent.Namespace = "test-namespace", POD_NAMESPACE = "instana-agent"
true,
logger,
mockDiscoverETCD,
)

require.NoError(t, err)
assert.Nil(
t,
deploymentContext,
"ETCD monitoring must be skipped when namespaces differ",
)
mockClient.AssertNotCalled(t, "Get")
mockClient.AssertNotCalled(t, "Apply")
},
)

t.Run("OpenShift skips ETCD monitoring when POD_NAMESPACE is not set", func(t *testing.T) {
require.NoError(t, os.Unsetenv("POD_NAMESPACE"))

mockClient := &mocks.MockInstanaAgentClient{}
mockDiscoverETCD := func(ctx context.Context, agent *instanav1.InstanaAgent) (*DiscoveredETCDTargets, error) {
return nil, nil
}
Expand All @@ -127,13 +192,13 @@ func TestCreateDeploymentContext_SimplifiedTests(t *testing.T) {
)

require.NoError(t, err)
require.NotNil(t, deploymentContext)
assert.True(
assert.Nil(
t,
deploymentContext.OpenShiftETCDResourcesExist,
"ETCD resources should exist when Get calls succeed",
deploymentContext,
"ETCD monitoring must be skipped when POD_NAMESPACE is unset",
)
mockClient.AssertExpectations(t)
mockClient.AssertNotCalled(t, "Get")
mockClient.AssertNotCalled(t, "Apply")
})

t.Run("Vanilla K8s with no ETCD returns nil", func(t *testing.T) {
Expand Down