Skip to content

Commit

Permalink
Merge pull request #5476 from slashpai/agent_validation
Browse files Browse the repository at this point in the history 
fix: Add validation method for controller and crd's
  • Loading branch information
@simonpasquier
simonpasquier committed Apr 27, 2023
2 parents f237294 + 096c4c2 commit e5cd5c5
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 65 deletions.
37 changes: 34 additions & 3 deletions cmd/operator/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
logging "github.com/prometheus-operator/prometheus-operator/internal/log"
"github.com/prometheus-operator/prometheus-operator/pkg/admission"
alertmanagercontroller "github.com/prometheus-operator/prometheus-operator/pkg/alertmanager"
monitoringv1alpha1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1alpha1"
"github.com/prometheus-operator/prometheus-operator/pkg/k8sutil"
"github.com/prometheus-operator/prometheus-operator/pkg/operator"
prometheusagentcontroller "github.com/prometheus-operator/prometheus-operator/pkg/prometheus/agent"
Expand Down Expand Up @@ -266,13 +267,41 @@ func Main() int {
return 1
}

pao, err := prometheusagentcontroller.New(ctx, cfg, log.With(logger, "component", "prometheusagentoperator"), r)
var pao *prometheusagentcontroller.Operator
verbs := map[string][]string{
monitoringv1alpha1.PrometheusAgentName: {"get", "list", "watch"},
fmt.Sprintf("%s/status", monitoringv1alpha1.PrometheusAgentName): {"update"},
}

cc, err := k8sutil.NewCRDChecker(cfg.Host, cfg.TLSInsecure, &cfg.TLSConfig)
if err != nil {
fmt.Fprint(os.Stderr, "instantiating prometheus-agent controller failed: ", err)
level.Error(logger).Log("msg", "failed to create new CRDChecker object ", "err", err)
cancel()
return 1
}

err = cc.CheckPrerequisites(ctx,
namespaces(cfg.Namespaces.AllowList).asSlice(),
verbs,
monitoringv1alpha1.SchemeGroupVersion.String(),
monitoringv1alpha1.PrometheusAgentName)

switch {
case errors.Is(err, k8sutil.ErrPrerequiresitesFailed):
level.Warn(logger).Log("msg", "Prometheus agent controller disabled because prerequisites not met", "err", err)
case err != nil:
level.Error(logger).Log("msg", "failed to check prerequisites for prometheus-agent controller ", "err", err)
cancel()
return 1
default:
pao, err = prometheusagentcontroller.New(ctx, cfg, log.With(logger, "component", "prometheusagentoperator"), r)
if err != nil {
level.Error(logger).Log("msg", "instantiating prometheus-agent controller failed", "err", err)
cancel()
return 1
}
}

ao, err := alertmanagercontroller.New(ctx, cfg, log.With(logger, "component", "alertmanageroperator"), r)
if err != nil {
fmt.Fprint(os.Stderr, "instantiating alertmanager controller failed: ", err)
Expand Down Expand Up @@ -360,7 +389,9 @@ func Main() int {
}))

wg.Go(func() error { return po.Run(ctx) })
wg.Go(func() error { return pao.Run(ctx) })
if pao != nil {
wg.Go(func() error { return pao.Run(ctx) })
}
wg.Go(func() error { return ao.Run(ctx) })
wg.Go(func() error { return to.Run(ctx) })

Expand Down
88 changes: 87 additions & 1 deletion pkg/k8sutil/k8sutil.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package k8sutil

import (
"context"
"errors"
"fmt"
"net/http"
"net/url"
Expand All @@ -25,18 +24,21 @@ import (
"strings"

"github.com/cespare/xxhash/v2"
"github.com/pkg/errors"
monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
monitoringv1alpha1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1alpha1"
monitoringv1beta1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1beta1"
promversion "github.com/prometheus/common/version"
appsv1 "k8s.io/api/apps/v1"
authv1 "k8s.io/api/authorization/v1"
v1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/client-go/discovery"
"k8s.io/client-go/kubernetes"
clientappsv1 "k8s.io/client-go/kubernetes/typed/apps/v1"
clientv1 "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/client-go/rest"
Expand All @@ -51,12 +53,31 @@ var invalidDNS1123Characters = regexp.MustCompile("[^-a-z0-9]+")

var scheme = runtime.NewScheme()

var ErrPrerequiresitesFailed = errors.New("unmet prerequisites")

func init() {
_ = monitoringv1.SchemeBuilder.AddToScheme(scheme)
_ = monitoringv1alpha1.SchemeBuilder.AddToScheme(scheme)
_ = monitoringv1beta1.SchemeBuilder.AddToScheme(scheme)
}

type CRDChecker struct {
kclient kubernetes.Interface
}

func NewCRDChecker(host string, tlsInsecure bool, tlsConfig *rest.TLSClientConfig) (*CRDChecker, error) {
cfg, err := NewClusterConfig(host, tlsInsecure, tlsConfig)
if err != nil {
return nil, errors.Wrap(err, "instantiating cluster config failed")
}

kclient, err := kubernetes.NewForConfig(cfg)
if err != nil {
return nil, errors.Wrap(err, "instantiating kubernetes client failed")
}
return &CRDChecker{kclient: kclient}, nil
}

// PodRunningAndReady returns whether a pod is running and each container has
// passed it's ready state.
func PodRunningAndReady(pod v1.Pod) (bool, error) {
Expand Down Expand Up @@ -114,6 +135,71 @@ func NewClusterConfig(host string, tlsInsecure bool, tlsConfig *rest.TLSClientCo
return cfg, nil
}

// CheckPrerequisites checks if given resource's CRD is installed in the cluster and the operator
// serviceaccount has the necessary RBAC verbs in the namespace list to reconcile it.
func (cc CRDChecker) CheckPrerequisites(ctx context.Context, nsAllowList []string, verbs map[string][]string, sgv, resource string) error {
if err := cc.validateCRDInstallation(sgv, resource); err != nil {
return err
}

missingPermissions, err := cc.getMissingPermissions(ctx, nsAllowList, verbs)
if err != nil {
return err
}
if len(missingPermissions) > 0 {
return fmt.Errorf("%w: some permissions are missing: %v", ErrPrerequiresitesFailed, missingPermissions)
}

return nil
}

func (cc CRDChecker) validateCRDInstallation(sgv, resource string) error {
crdInstalled, err := IsAPIGroupVersionResourceSupported(cc.kclient.Discovery(), sgv, resource)
if err != nil {
return fmt.Errorf("failed to check if the API supports %s resource (apiGroup: %q)", resource, sgv)
}
if !crdInstalled {
return fmt.Errorf("%w: %s resource (apiGroup: %q) not installed", ErrPrerequiresitesFailed, resource, sgv)
}
return nil
}

// getMissingPermissions returns the RBAC permissions that the controller would need to be
// granted to fulfill its mission. An empty map means that everything is ok.
func (cc CRDChecker) getMissingPermissions(ctx context.Context, nsAllowList []string, verbs map[string][]string) (map[string][]string, error) {
var ssar *authv1.SelfSubjectAccessReview
var ssarResponse *authv1.SelfSubjectAccessReview
var err error
missingPermissions := map[string][]string{}

for _, ns := range nsAllowList {
for resource, verbs := range verbs {
for _, verb := range verbs {
ssar = &authv1.SelfSubjectAccessReview{
Spec: authv1.SelfSubjectAccessReviewSpec{
ResourceAttributes: &authv1.ResourceAttributes{
Verb: verb,
Group: monitoringv1alpha1.SchemeGroupVersion.Group,
Resource: resource,
// If ns is empty string, it will check cluster-wide
Namespace: ns,
},
},
}
ssarResponse, err = cc.kclient.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, ssar, metav1.CreateOptions{})
if err != nil {
return nil, err
}
if !ssarResponse.Status.Allowed {
missingPermissions[resource] = append(missingPermissions[resource], verb)
}
}
}
}

return missingPermissions, nil
}

func IsResourceNotFoundError(err error) bool {
se, ok := err.(*apierrors.StatusError)
if !ok {
Expand Down
61 changes: 0 additions & 61 deletions pkg/prometheus/agent/operator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import (
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
appsv1 "k8s.io/api/apps/v1"
authv1 "k8s.io/api/authorization/v1"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -282,25 +281,6 @@ func New(ctx context.Context, conf operator.Config, logger log.Logger, r prometh

// Run the controller.
func (c *Operator) Run(ctx context.Context) error {
crdInstalled, err := k8sutil.IsAPIGroupVersionResourceSupported(c.kclient.Discovery(), monitoringv1alpha1.SchemeGroupVersion.String(), monitoringv1alpha1.PrometheusAgentName)
if err != nil {
level.Warn(c.logger).Log("msg", "failed to check if the API supports the PrometheusAgent CRD", "err ", err)
return nil
}
if !crdInstalled {
level.Info(c.logger).Log("msg", "Prometheus agent controller disabled because the PrometheusAgent CRD isn't installed")
return nil
}

missingPermissions, err := c.getMissingPermissions(ctx)
if err != nil {
return err
}
if len(missingPermissions) > 0 {
level.Warn(c.logger).Log("msg", "Prometheus agent controller disabled because it lacks the required permissions on PrometheusAgent objects", "missingpermissions", fmt.Sprintf("%v", missingPermissions))
return nil
}

errChan := make(chan error)
go func() {
v, err := c.kclient.Discovery().ServerVersion()
Expand Down Expand Up @@ -1284,44 +1264,3 @@ func (c *Operator) handleMonitorNamespaceUpdate(oldo, curo interface{}) {
)
}
}

// getMissingPermissions returns the RBAC permissions that the controller would need to be
// granted to fulfill its mission. An empty map means that everything is ok.
func (c *Operator) getMissingPermissions(ctx context.Context) (map[string][]string, error) {
verbs := map[string][]string{
monitoringv1alpha1.PrometheusAgentName: {"get", "list", "watch"},
fmt.Sprintf("%s/status", monitoringv1alpha1.PrometheusAgentName): {"update"},
}
var ssar *authv1.SelfSubjectAccessReview
var ssarResponse *authv1.SelfSubjectAccessReview
var err error

missingPermissions := map[string][]string{}

for ns := range c.config.Namespaces.PrometheusAllowList {
for resource, verbs := range verbs {
for _, verb := range verbs {
ssar = &authv1.SelfSubjectAccessReview{
Spec: authv1.SelfSubjectAccessReviewSpec{
ResourceAttributes: &authv1.ResourceAttributes{
Verb: verb,
Group: monitoringv1alpha1.SchemeGroupVersion.Group,
Resource: resource,
// If ns is empty string, it will check cluster-wide
Namespace: ns,
},
},
}
ssarResponse, err = c.kclient.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, ssar, metav1.CreateOptions{})
if err != nil {
return nil, err
}
if !ssarResponse.Status.Allowed {
missingPermissions[resource] = append(missingPermissions[resource], verb)
}
}
}
}

return missingPermissions, nil
}

0 comments on commit e5cd5c5

Please sign in to comment.