Skip to content

Commit

Permalink
Adjust code in order to remove operator RBAC permissions for namespaces
Browse files Browse the repository at this point in the history
We grant namespace read permissions which is only needed to query for
the existence of the "openshift-monitoring" namespace to determine
where to create ServiceMonitors. However we can eliminate this query
and thus the permissions by trying to create ServiceMonitors in the
"openshift-monitoring" namespace and, if the error indicates the
namespace is missing, create in the provided namespace.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
  • Loading branch information
tpantelis committed May 4, 2024
1 parent a251579 commit 2691550
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 44 deletions.
9 changes: 0 additions & 9 deletions config/rbac/submariner-operator/cluster_role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,6 @@ rules:
verbs:
- get
- list
- apiGroups:
- ""
resources:
# Needed for openshift monitoring
- namespaces
verbs:
- get
- list
- watch
- apiGroups:
- monitoring.coreos.com
resources:
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ require (
github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.73.2
github.com/prometheus-operator/prometheus-operator/pkg/client v0.73.2
github.com/prometheus/client_golang v1.19.0
github.com/submariner-io/admiral v0.18.0-m3
github.com/submariner-io/admiral v0.18.0-m3.0.20240504143439-22ee51817b45
github.com/submariner-io/shipyard v0.18.0-m3
github.com/submariner-io/submariner v0.18.0-m3
golang.org/x/net v0.24.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,8 @@ github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81P
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
github.com/submariner-io/admiral v0.18.0-m3 h1:L87Jwb4oYAHbfuHxJ0BlKucZT5lP/OdyAuS18NenS6Q=
github.com/submariner-io/admiral v0.18.0-m3/go.mod h1:vkFQs9wWq1HY4k3JesxlLfKZTaA/llOkbdCDZCdk3yc=
github.com/submariner-io/admiral v0.18.0-m3.0.20240504143439-22ee51817b45 h1:7wwl7v808XgYCCs1fKXhQHYCA+njJLmWyNekbiefSFg=
github.com/submariner-io/admiral v0.18.0-m3.0.20240504143439-22ee51817b45/go.mod h1:vkFQs9wWq1HY4k3JesxlLfKZTaA/llOkbdCDZCdk3yc=
github.com/submariner-io/shipyard v0.18.0-m3 h1:N0/BAwTv5p6O7PgvQeouUzcgybJtq7QQ66gjos7+F48=
github.com/submariner-io/shipyard v0.18.0-m3/go.mod h1:qs1LOCrPfM6H3JzR8TWNXFW4hvBiY+8gJ6OOjF4o4E0=
github.com/submariner-io/submariner v0.18.0-m3 h1:IVpsPwFHLc1AK4/Ga8GtgdXVxnu3w7SfmoEgprmrcOw=
Expand Down
9 changes: 0 additions & 9 deletions pkg/embeddedyamls/yamls.go
Original file line number Diff line number Diff line change
Expand Up @@ -2755,15 +2755,6 @@ rules:
verbs:
- get
- list
- apiGroups:
- ""
resources:
# Needed for openshift monitoring
- namespaces
verbs:
- get
- list
- watch
- apiGroups:
- monitoring.coreos.com
resources:
Expand Down
33 changes: 10 additions & 23 deletions pkg/metrics/service-monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,15 @@ import (
"github.com/pkg/errors"
monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
monclientv1 "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned/typed/monitoring/v1"
"github.com/submariner-io/admiral/pkg/resource"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/discovery"
clientset "k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
"k8s.io/utils/ptr"
logf "sigs.k8s.io/controller-runtime/pkg/log"
)

var (
log = logf.Log.WithName("metrics")
ErrServiceMonitorNotPresent = fmt.Errorf("no ServiceMonitor registered with the API")
)
var ErrServiceMonitorNotPresent = fmt.Errorf("no ServiceMonitor registered with the API")

const openshiftMonitoringNS = "openshift-monitoring"

Expand All @@ -57,20 +52,6 @@ func CreateServiceMonitors(ctx context.Context, config *rest.Config, ns string,
return nil, ErrServiceMonitorNotPresent
}

// On OpenShift, we need to create the service monitors in the OpenShift monitoring namespace, not the
// services; we need our own clientset rather than the manager's since the latter hasn't started yet
// (so its caching infrastructure isn't available, and reads fail)
cs, err := clientset.NewForConfig(config)
if err != nil {
return nil, errors.Wrap(err, "error getting kube client")
}

if _, err := cs.CoreV1().Namespaces().Get(ctx, openshiftMonitoringNS, metav1.GetOptions{}); err == nil {
ns = openshiftMonitoringNS
} else if !apierrors.IsNotFound(err) {
log.Error(err, "Error checking for the OpenShift monitoring namespace")
}

serviceMonitors := make([]*monitoringv1.ServiceMonitor, len(services))
mclient := monclientv1.NewForConfigOrDie(config)

Expand All @@ -79,9 +60,15 @@ func CreateServiceMonitors(ctx context.Context, config *rest.Config, ns string,
continue
}

sm := GenerateServiceMonitor(ns, s)
// On OpenShift, we need to create the service monitors in the OpenShift monitoring namespace, not the
// service's. If that namespace doesn't exist then create in the provided namespace.
smc, err := mclient.ServiceMonitors(ns).Create(ctx, GenerateServiceMonitor(openshiftMonitoringNS, s), metav1.CreateOptions{})

missingNS, _ := resource.IsMissingNamespaceErr(err)
if missingNS {
smc, err = mclient.ServiceMonitors(ns).Create(ctx, GenerateServiceMonitor(ns, s), metav1.CreateOptions{})
}

smc, err := mclient.ServiceMonitors(ns).Create(ctx, sm, metav1.CreateOptions{})
if err != nil {
return nil, errors.Wrap(err, "error creating ServiceMonitor")
}
Expand Down

0 comments on commit 2691550

Please sign in to comment.