Skip to content

Allow for scoping TA to watch namespace #3763

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

Merged
merged 1 commit into from
Mar 18, 2025
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
19 changes: 19 additions & 0 deletions .chloggen/namespace-ta.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
component: target allocator

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: |
Add support for setting the allowNamespaces and denyNamespaces in the target allocator.

# One or more tracking issues related to the change
issues: [3086]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
allowNamespaces can be set to an empty list to watch all namespaces (default) or to list of namespaces to watch.
denyNamespaces can be set to an empty list to deny watching any namespaces (default) or to a list of namespaces to deny watching.
5 changes: 5 additions & 0 deletions apis/v1alpha1/targetallocator_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ func (w TargetAllocatorWebhook) validate(ctx context.Context, ta *TargetAllocato
if err != nil || len(warnings) > 0 {
return warnings, err
}

// Check to see that allowNamespaces and denyNamespaces are not both set at the same time
if len(ta.Spec.PrometheusCR.AllowNamespaces) > 0 && len(ta.Spec.PrometheusCR.DenyNamespaces) > 0 {
return warnings, fmt.Errorf("allowNamespaces and denyNamespaces are mutually exclusive")
}
}

return warnings, nil
Expand Down
13 changes: 13 additions & 0 deletions apis/v1alpha1/targetallocator_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,19 @@ func TestTargetAllocatorValidatingWebhook(t *testing.T) {
},
expectedErr: "the OpenTelemetry Spec Ports configuration is incorrect",
},
{
name: "allowNamespaces and denyNamespaces can't both be set",
targetallocator: TargetAllocator{
Spec: TargetAllocatorSpec{
PrometheusCR: v1beta1.TargetAllocatorPrometheusCR{
Enabled: true,
AllowNamespaces: []string{"ns1"},
DenyNamespaces: []string{"ns2"},
},
},
},
expectedErr: "allowNamespaces and denyNamespaces are mutually exclusive",
},
}

for _, test := range tests {
Expand Down
6 changes: 6 additions & 0 deletions apis/v1beta1/targetallocator_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ type TargetAllocatorPrometheusCR struct {
// Enabled indicates whether to use a PrometheusOperator custom resources as targets or not.
// +optional
Enabled bool `json:"enabled,omitempty"`
// AllowNamespaces Namespaces to scope the interaction of the Target Allocator and the apiserver (allow list). This is mutually exclusive with DenyNamespaces.
// +optional
AllowNamespaces []string `json:"allowNamespaces,omitempty"`
// DenyNamespaces Namespaces to scope the interaction of the Target Allocator and the apiserver (deny list). This is mutually exclusive with AllowNamespaces.
// +optional
DenyNamespaces []string `json:"denyNamespaces,omitempty"`
// Default interval between consecutive scrapes. Intervals set in ServiceMonitors and PodMonitors override it.
//Equivalent to the same setting on the Prometheus CR.
//
Expand Down
10 changes: 10 additions & 0 deletions apis/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -7889,6 +7889,14 @@ spec:
type: object
prometheusCR:
properties:
allowNamespaces:
items:
type: string
type: array
denyNamespaces:
items:
type: string
type: array
enabled:
type: boolean
podMonitorSelector:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2257,6 +2257,14 @@ spec:
type: string
prometheusCR:
properties:
allowNamespaces:
items:
type: string
type: array
denyNamespaces:
items:
type: string
type: array
enabled:
type: boolean
podMonitorSelector:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7889,6 +7889,14 @@ spec:
type: object
prometheusCR:
properties:
allowNamespaces:
items:
type: string
type: array
denyNamespaces:
items:
type: string
type: array
enabled:
type: boolean
podMonitorSelector:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2257,6 +2257,14 @@ spec:
type: string
prometheusCR:
properties:
allowNamespaces:
items:
type: string
type: array
denyNamespaces:
items:
type: string
type: array
enabled:
type: boolean
podMonitorSelector:
Expand Down
91 changes: 83 additions & 8 deletions cmd/otel-allocator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,11 @@ Upstream documentation here: [PrometheusReceiver](https://github.com/open-teleme

### RBAC

Before the TargetAllocator can start scraping, you need to set up Kubernetes RBAC (role-based access controls) resources. This means that you need to have a `ServiceAccount` and corresponding cluster roles so that the TargetAllocator has access to all of the necessary resources to pull metrics from.
Before the TargetAllocator can start scraping, you need to set up Kubernetes RBAC (role-based access controls) resources. This means that you need to have a `ServiceAccount` and corresponding ClusterRoles/Roles so that the TargetAllocator has access to all the necessary resources to pull metrics from.

You can create your own `ServiceAccount`, and reference it in `spec.targetAllocator.serviceAccount` in your `OpenTelemetryCollector` CR. You’ll then need to configure the `ClusterRole` and `ClusterRoleBinding` for this `ServiceAccount`, as per below.
You can create your own `ServiceAccount`, and reference it in `spec.targetAllocator.serviceAccount` in your `OpenTelemetryCollector` CR. You’ll then need to configure the `ClusterRole` and `ClusterRoleBinding` or `Role` and `RoleBinding` for this `ServiceAccount`, as per below.

#### Cluster-scoped RBAC

```yaml
targetAllocator:
Expand All @@ -204,11 +206,11 @@ You can create your own `ServiceAccount`, and reference it in `spec.targetAlloca
```

> 🚨 **Note**: The Collector part of this same CR *also* has a serviceAccount key which only affects the collector and *not*
the TargetAllocator.
> the TargetAllocator.

If you omit the `ServiceAccount` name, the TargetAllocator creates a `ServiceAccount` for you. The `ServiceAccount`’s default name is a concatenation of the Collector name and the `-targetallocator` suffix. By default, this `ServiceAccount` has no defined policy, so you’ll need to create your own `ClusterRole` and `ClusterRoleBinding` for it, as per below.
If you omit the `ServiceAccount` name, the TargetAllocator creates a `ServiceAccount` for you. The `ServiceAccount`’s default name is a concatenation of the Collector name and the `-targetallocator` suffix. By default, this `ServiceAccount` has no defined policy, so you’ll need to create your own `ClusterRole` and `ClusterRoleBinding` or `Role` and `RoleBinding` for it, as per below.

The role below will provide the minimum access required for the Target Allocator to query all the targets it needs based on any Prometheus configurations:
The ClusterRole below will provide the minimum access required for the Target Allocator to query all the targets it needs based on any Prometheus configurations:

```yaml
apiVersion: rbac.authorization.k8s.io/v1
Expand Down Expand Up @@ -242,7 +244,7 @@ rules:
verbs: ["get"]
```

If you enable the the `prometheusCR` (set `spec.targetAllocator.prometheusCR.enabled` to `true`) in the `OpenTelemetryCollector` CR, you will also need to define the following roles. These give the TargetAllocator access to the `PodMonitor` and `ServiceMonitor` CRs. It also gives namespace access to the `PodMonitor` and `ServiceMonitor`.
If you enable the `prometheusCR` (set `spec.targetAllocator.prometheusCR.enabled` to `true`) in the `OpenTelemetryCollector` CR, you will also need to define the following ClusterRoles. These give the TargetAllocator access to the `PodMonitor` and `ServiceMonitor` CRs. It also gives namespace access to the `PodMonitor` and `ServiceMonitor`.

```yaml
apiVersion: rbac.authorization.k8s.io/v1
Expand All @@ -263,8 +265,82 @@ rules:
verbs: ["get", "list", "watch"]
```

> ✨ The above roles can be combined into a single role.
> ✨ The above ClusterRoles can be combined into a single ClusterRole.

#### Namespace-scoped RBAC

If you want to have the TargetAllocator watch a specific namespace, you can set the allowNamespaces field
in the TargetAllocator's prometheusCR configuration. This is useful if you want to restrict the TargetAllocator to only watch Prometheus
CRs in a specific namespace, and not have cluster-wide access.

```yaml
targetAllocator:
enabled: true
serviceAccount: opentelemetry-targetallocator-sa
prometheusCR:
enabled: true
allowNamespaces:
- foo
```

In this case, you will need to create a Role and RoleBinding instead of a ClusterRole and ClusterRoleBinding. The Role
and RoleBinding should be created in the namespace specified by the allowNamespaces field.
Comment on lines +286 to +287
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind writing an issue for a follow up to use our auto-rbac system to make these for users if they've enabled the operator to do so?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created the issue here: #3816


```yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: opentelemetry-targetallocator-role
rules:
- apiGroups:
- ""
resources:
- pods
- services
- endpoints
- configmaps
- secrets
- namespaces
verbs:
- get
- watch
- list
- apiGroups:
- apps
resources:
- statefulsets
verbs:
- get
- watch
- list
- apiGroups:
- discovery.k8s.io
resources:
- endpointslices
verbs:
- get
- watch
- list
- apiGroups:
- networking.k8s.io
resources:
- ingresses
verbs:
- get
- watch
- list
- apiGroups:
- monitoring.coreos.com
resources:
- servicemonitors
- podmonitors
- scrapeconfigs
- probes
verbs:
- get
- watch
- list
```

### Service / Pod monitor endpoint credentials

Expand Down Expand Up @@ -420,4 +496,3 @@ Shards the received targets based on the discovered Collector instances

### Collector
Client to watch for deployed Collector instances which will then provided to the Allocator.

28 changes: 28 additions & 0 deletions cmd/otel-allocator/internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
_ "github.com/prometheus/prometheus/discovery/install"
"github.com/spf13/pflag"
"gopkg.in/yaml.v2"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
Expand Down Expand Up @@ -53,6 +54,8 @@ type Config struct {

type PrometheusCRConfig struct {
Enabled bool `yaml:"enabled,omitempty"`
AllowNamespaces []string `yaml:"allow_namespaces,omitempty"`
DenyNamespaces []string `yaml:"deny_namespaces,omitempty"`
PodMonitorSelector *metav1.LabelSelector `yaml:"pod_monitor_selector,omitempty"`
PodMonitorNamespaceSelector *metav1.LabelSelector `yaml:"pod_monitor_namespace_selector,omitempty"`
ServiceMonitorSelector *metav1.LabelSelector `yaml:"service_monitor_selector,omitempty"`
Expand Down Expand Up @@ -342,6 +345,9 @@ func ValidateConfig(config *Config) error {
if config.CollectorNamespace == "" {
return fmt.Errorf("collector namespace must be set")
}
if len(config.PrometheusCR.AllowNamespaces) != 0 && len(config.PrometheusCR.DenyNamespaces) != 0 {
return fmt.Errorf("only one of allowNamespaces or denyNamespaces can be set")
}
return nil
}

Expand All @@ -367,3 +373,25 @@ func (c HTTPSServerConfig) NewTLSConfig() (*tls.Config, error) {
}
return tlsConfig, nil
}

// GetAllowDenyLists returns the allow and deny lists as maps. If the allow list is empty, it defaults to all namespaces.
// If the deny list is empty, it defaults to an empty map.
func (c PrometheusCRConfig) GetAllowDenyLists() (map[string]struct{}, map[string]struct{}) {
allowList := map[string]struct{}{}
if len(c.AllowNamespaces) != 0 {
for _, ns := range c.AllowNamespaces {
allowList[ns] = struct{}{}
}
} else {
allowList = map[string]struct{}{v1.NamespaceAll: {}}
}

denyList := map[string]struct{}{}
if len(c.DenyNamespaces) != 0 {
for _, ns := range c.DenyNamespaces {
denyList[ns] = struct{}{}
}
}

return allowList, denyList
}
56 changes: 56 additions & 0 deletions cmd/otel-allocator/internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/prometheus/prometheus/discovery/file"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -538,6 +539,18 @@ func TestValidateConfig(t *testing.T) {
},
expectedErr: nil,
},
{
name: "both allowNamespaces and denyNamespaces set",
fileConfig: Config{
PrometheusCR: PrometheusCRConfig{
Enabled: true,
AllowNamespaces: []string{"ns1"},
DenyNamespaces: []string{"ns2"},
},
CollectorNamespace: "default",
},
expectedErr: fmt.Errorf("only one of allowNamespaces or denyNamespaces can be set"),
},
}

for _, tc := range testCases {
Expand All @@ -548,3 +561,46 @@ func TestValidateConfig(t *testing.T) {
})
}
}

func TestGetAllowDenyLists(t *testing.T) {
testCases := []struct {
name string
promCRConfig PrometheusCRConfig
expectedAllowList map[string]struct{}
expectedDenyList map[string]struct{}
}{
{
name: "no allow or deny namespaces",
promCRConfig: PrometheusCRConfig{Enabled: true},
expectedAllowList: map[string]struct{}{v1.NamespaceAll: {}},
expectedDenyList: map[string]struct{}{},
},
{
name: "allow namespaces",
promCRConfig: PrometheusCRConfig{Enabled: true, AllowNamespaces: []string{"ns1"}},
expectedAllowList: map[string]struct{}{"ns1": {}},
expectedDenyList: map[string]struct{}{},
},
{
name: "deny namespaces",
promCRConfig: PrometheusCRConfig{Enabled: true, DenyNamespaces: []string{"ns2"}},
expectedAllowList: map[string]struct{}{v1.NamespaceAll: {}},
expectedDenyList: map[string]struct{}{"ns2": {}},
},
{
name: "both allow and deny namespaces",
promCRConfig: PrometheusCRConfig{Enabled: true, AllowNamespaces: []string{"ns1"}, DenyNamespaces: []string{"ns2"}},
expectedAllowList: map[string]struct{}{"ns1": {}},
expectedDenyList: map[string]struct{}{"ns2": {}},
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
allowList, denyList := tc.promCRConfig.GetAllowDenyLists()
assert.Equal(t, tc.expectedAllowList, allowList)
assert.Equal(t, tc.expectedDenyList, denyList)
})
}
}
Loading
Loading