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

fix: Add validation method for controller and crd's #5476

Merged
merged 1 commit into from
Apr 27, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 34 additions & 3 deletions cmd/operator/main.go
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"},
}
simonpasquier marked this conversation as resolved.
Show resolved Hide resolved

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
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
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)
Copy link
Contributor Author

@slashpai slashpai Apr 19, 2023

Choose a reason for hiding this comment

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

Moved to k8sutil.CheckPrerequisites()

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 @@ -1332,44 +1312,3 @@ func (c *Operator) handleMonitorNamespaceUpdate(oldo, curo interface{}) {
)
}
}

// getMissingPermissions returns the RBAC permissions that the controller would need to be
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to k8sutil

// 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
}