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

Automate the creation of the permissions needed by resourcedetection #2394

Merged
merged 30 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
cb4771a
Automate the creation of the permissions requested by resourcedetection
iblancasa Nov 28, 2023
c566e26
Add changelog
iblancasa Nov 28, 2023
d90814d
Fix merge
iblancasa Nov 28, 2023
d6444ab
Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…
iblancasa Dec 4, 2023
c695885
Apply changes requested in code review
iblancasa Dec 4, 2023
bae77d2
Merge branch 'main' into task/2393
iblancasa Dec 4, 2023
a977949
Fix lint
iblancasa Dec 5, 2023
921b6eb
Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…
iblancasa Dec 5, 2023
7aa647a
Merge branch 'task/2393' of github.com:iblancasa/opentelemetry-operat…
iblancasa Dec 5, 2023
6bd7d5f
Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…
iblancasa Dec 18, 2023
10f9ea2
Add feature gate and test
iblancasa Dec 18, 2023
b3243d2
Merge branch 'main' into task/2393
iblancasa Dec 18, 2023
23531b6
Merge branch 'main' into task/2393
iblancasa Dec 19, 2023
ac325d2
Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…
iblancasa Dec 20, 2023
1e70aea
Add unit tests
iblancasa Dec 20, 2023
be7042a
Merge branch 'task/2393' of github.com:iblancasa/opentelemetry-operat…
iblancasa Dec 20, 2023
4ef9f74
Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…
iblancasa Jan 2, 2024
d872607
Apply feedback from pull request
iblancasa Jan 2, 2024
461cd29
Merge branch 'main' into task/2393
iblancasa Jan 4, 2024
af31a85
Merge branch 'main' into task/2393
iblancasa Jan 4, 2024
70badae
Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…
iblancasa Jan 4, 2024
78acfa0
Merge branch 'task/2393' of github.com:iblancasa/opentelemetry-operat…
iblancasa Jan 8, 2024
ddd8194
Apply changes requested as part of the Pull Request
iblancasa Jan 8, 2024
d83b683
Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…
iblancasa Jan 8, 2024
29f4ca6
Merge branch 'main' into task/2393
iblancasa Jan 9, 2024
40ee3ef
Merge branch 'main' into task/2393
iblancasa Jan 10, 2024
9fd612d
Merge branch 'task/2393' of github.com:iblancasa/opentelemetry-operat…
iblancasa Jan 11, 2024
c99f53f
Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…
iblancasa Jan 11, 2024
a3d9d95
Apply changes requested as part of the Pull Request
iblancasa Jan 11, 2024
132328b
Merge branch 'main' into task/2393
iblancasa Jan 11, 2024
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
16 changes: 16 additions & 0 deletions .chloggen/2393-automate-permissions-resourcedetection.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# 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. operator, target allocator, github action)
component: operator

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Automate the creation of the permissions needed by the resourcedetection processor

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

# (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:
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ prepare-e2e: kuttl set-image-controller container container-target-allocator con
enable-prometheus-feature-flag:
$(SED) -i "s#--feature-gates=+operator.autoinstrumentation.go#--feature-gates=+operator.autoinstrumentation.go,+operator.observability.prometheus#g" config/default/manager_auth_proxy_patch.yaml


.PHONY: scorecard-tests
scorecard-tests: operator-sdk
$(OPERATOR_SDK) scorecard -w=5m bundle || (echo "scorecard test failed" && exit 1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,15 @@ spec:
- patch
- update
- watch
- apiGroups:
- config.openshift.io
resources:
- infrastructures
- infrastructures/status
verbs:
- get
- list
- watch
- apiGroups:
- coordination.k8s.io
resources:
Expand Down Expand Up @@ -342,6 +351,19 @@ spec:
- patch
- update
- watch
- apiGroups:
- rbac.authorization.k8s.io
resources:
- clusterrolebindings
- clusterroles
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- route.openshift.io
resources:
Expand Down
22 changes: 22 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ rules:
- patch
- update
- watch
- apiGroups:
- config.openshift.io
resources:
- infrastructures
- infrastructures/status
verbs:
- get
- list
- watch
- apiGroups:
- coordination.k8s.io
resources:
Expand Down Expand Up @@ -175,6 +184,19 @@ rules:
- patch
- update
- watch
- apiGroups:
- rbac.authorization.k8s.io
resources:
- clusterrolebindings
- clusterroles
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- route.openshift.io
resources:
Expand Down
9 changes: 8 additions & 1 deletion controllers/opentelemetrycollector_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
autoscalingv2 "k8s.io/api/autoscaling/v2"
corev1 "k8s.io/api/core/v1"
policyV1 "k8s.io/api/policy/v1"
rbacv1 "k8s.io/api/rbac/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/record"
Expand Down Expand Up @@ -83,10 +84,12 @@ func NewReconciler(p Params) *OpenTelemetryCollectorReconciler {
// +kubebuilder:rbac:groups=apps,resources=daemonsets;deployments;statefulsets,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=autoscaling,resources=horizontalpodautoscalers,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings;clusterroles,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;create;update
// +kubebuilder:rbac:groups=monitoring.coreos.com,resources=servicemonitors;podmonitors,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=route.openshift.io,resources=routes;routes/custom-host,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=config.openshift.io,resources=infrastructures;infrastructures/status,verbs=get;list;watch
jaronoff97 marked this conversation as resolved.
Show resolved Hide resolved
// +kubebuilder:rbac:groups=opentelemetry.io,resources=opentelemetrycollectors,verbs=get;list;watch;update;patch
// +kubebuilder:rbac:groups=opentelemetry.io,resources=opentelemetrycollectors/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=opentelemetry.io,resources=opentelemetrycollectors/finalizers,verbs=get;update;patch
Expand Down Expand Up @@ -138,7 +141,11 @@ func (r *OpenTelemetryCollectorReconciler) SetupWithManager(mgr ctrl.Manager) er
Owns(&appsv1.DaemonSet{}).
Owns(&appsv1.StatefulSet{}).
Owns(&autoscalingv2.HorizontalPodAutoscaler{}).
Owns(&policyV1.PodDisruptionBudget{})
Owns(&policyV1.PodDisruptionBudget{}).
Owns(&autoscalingv2.HorizontalPodAutoscaler{}).
iblancasa marked this conversation as resolved.
Show resolved Hide resolved
Owns(&policyV1.PodDisruptionBudget{}).
iblancasa marked this conversation as resolved.
Show resolved Hide resolved
Owns(&rbacv1.ClusterRoleBinding{}).
Owns(&rbacv1.ClusterRole{})

if featuregate.PrometheusOperatorIsAvailable.IsEnabled() {
builder.Owns(&monitoringv1.ServiceMonitor{})
Expand Down
5 changes: 4 additions & 1 deletion internal/manifests/collector/adapters/config_to_ports.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ type ComponentType int
const (
ComponentTypeReceiver ComponentType = iota
ComponentTypeExporter
ComponentTypeProcessor
)

func (c ComponentType) String() string {
return [...]string{"receiver", "exporter"}[c]
return [...]string{"receiver", "exporter", "processor"}[c]
}

// ConfigToComponentPorts converts the incoming configuration object into a set of service ports required by the exporters.
Expand Down Expand Up @@ -94,6 +95,8 @@ func ConfigToComponentPorts(logger logr.Logger, cType ComponentType, config map[
cmptParser, err = exporterParser.For(logger, cmptName, exporter)
case ComponentTypeReceiver:
cmptParser, err = receiverParser.For(logger, cmptName, exporter)
case ComponentTypeProcessor:
logger.V(4).Info("processors don't provide a way to enable associated ports", "name", key)
}

if err != nil {
Expand Down
62 changes: 62 additions & 0 deletions internal/manifests/collector/adapters/config_to_rbac.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package adapters

import (
"github.com/go-logr/logr"
rbacv1 "k8s.io/api/rbac/v1"

"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/parser/processor"
)

func ConfigToRBAC(logger logr.Logger, config map[interface{}]interface{}) []rbacv1.PolicyRule {
Copy link
Member

Choose a reason for hiding this comment

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

nit: it's worth adding a comment to a public API

var policyRules []rbacv1.PolicyRule
processorsRaw, ok := config["processors"]
if !ok {
logger.V(2).Info("no processors available as part of the configuration")
return policyRules
}

processors, ok := processorsRaw.(map[interface{}]interface{})
if !ok {
logger.V(2).Info("processors doesn't contain valid components")
return policyRules
}

enabledProcessors := getEnabledComponents(config, ComponentTypeProcessor)

for key, val := range processors {
if !enabledProcessors[key] {
continue
}

processorCfg, ok := val.(map[interface{}]interface{})
if !ok {
logger.V(2).Info("processor doesn't seem to be a map of properties", "processor", key)
processorCfg = map[interface{}]interface{}{}
}

processorName := key.(string)
processorParser, err := processor.For(logger, processorName, processorCfg)
if err != nil {
logger.V(2).Info("no parser found for '%s'", processorName)
continue
}

policyRules = append(policyRules, processorParser.GetRBACRules()...)
}

return policyRules
}
98 changes: 98 additions & 0 deletions internal/manifests/collector/adapters/config_to_rbac_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package adapters

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
rbacv1 "k8s.io/api/rbac/v1"
logf "sigs.k8s.io/controller-runtime/pkg/log"
)

func TestConfigRBAC(t *testing.T) {
tests := []struct {
desc string
config string
expectedRules []rbacv1.PolicyRule
}{
{
desc: "No processors",
config: `processors:
service:
traces:
processors:`,
expectedRules: ([]rbacv1.PolicyRule)(nil),
},
{
desc: "processors no rbac",
config: `processors:
batch:
service:
pipelines:
traces:
processors: [batch]`,
expectedRules: ([]rbacv1.PolicyRule)(nil),
},
{
desc: "resourcedetection-processor k8s",
config: `processors:
resourcedetection:
detectors: [kubernetes]
service:
pipelines:
traces:
processors: [resourcedetection]`,
expectedRules: []rbacv1.PolicyRule{
{
APIGroups: []string{""},
Resources: []string{"nodes"},
Verbs: []string{"get", "list"},
},
},
},
{
desc: "resourcedetection-processor openshift",
config: `processors:
resourcedetection:
detectors: [openshift]
service:
pipelines:
traces:
processors: [resourcedetection]`,
expectedRules: []rbacv1.PolicyRule{
{
APIGroups: []string{"config.openshift.io"},
Resources: []string{"infrastructures", "infrastructures/status"},
Verbs: []string{"get", "watch", "list"},
},
},
},
}

var logger = logf.Log.WithName("collector-unit-tests")
for _, test := range tests {
Copy link
Member

Choose a reason for hiding this comment

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

It's good that the test table is used but the tests should be run in a separate execution t.Run

// prepare
config, err := ConfigFromString(test.config)
require.NoError(t, err, test.desc)
require.NotEmpty(t, config, test.desc)

// test
rules := ConfigToRBAC(logger, config)
assert.NoError(t, err)
assert.Equal(t, test.expectedRules, rules, test.desc)
}
}
2 changes: 1 addition & 1 deletion internal/manifests/collector/adapters/config_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import "fmt"
// Following Otel Doc: Configuring a receiver does not enable it. The receivers are enabled via pipelines within the service section.
// getEnabledComponents returns all enabled components as a true flag set. If it can't find any receiver, it will return a nil interface.
func getEnabledComponents(config map[interface{}]interface{}, componentType ComponentType) map[interface{}]bool {
componentTypePlural := fmt.Sprintf("%ss", componentType)
componentTypePlural := fmt.Sprintf("%ss", componentType.String())
cfgComponents, ok := config[componentTypePlural]
if !ok {
return nil
Expand Down
9 changes: 9 additions & 0 deletions internal/manifests/collector/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,22 @@ func Build(params manifests.Params) ([]client.Object, error) {
manifests.Factory(MonitoringService),
manifests.Factory(Ingress),
}...)

if params.OtelCol.Spec.Observability.Metrics.EnableMetrics && featuregate.PrometheusOperatorIsAvailable.IsEnabled() {
if params.OtelCol.Spec.Mode == v1alpha1.ModeSidecar {
manifestFactories = append(manifestFactories, manifests.Factory(PodMonitor))
} else {
manifestFactories = append(manifestFactories, manifests.Factory(ServiceMonitor))
}
}

if !featuregate.CreateRBACPermissions.IsEnabled() {
manifestFactories = append(manifestFactories,
manifests.FactoryWithoutError(ClusterRole),
manifests.FactoryWithoutError(ClusterRoleBinding),
)
}

for _, factory := range manifestFactories {
res, err := factory(params)
if err != nil {
Expand Down
Loading
Loading