add metrics to track RBAC adoption (#1381)

metrics/metrics.go

@@ -0,0 +1,37 @@
+package metrics
+
+import (
+	"github.com/RedHatInsights/insights-operator-utils/metrics"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
+)
+
+var (
+	rbacIdentityTypeOps = prometheus.CounterOpts{
+		Name: "rbac_identity_type",
+		Help: "The total number of requesters by identity type",
+	}
+
+	rbacServiceAccountRejectedOps = prometheus.CounterOpts{
+		Name: "rbac_service_accounts_rejected",
+		Help: "The total number of service accounts that were rejected due to ACL",
+	}
+)
+
+// RBACIdentityType shows number of requesters by identity type. For example
+// User, ServiceAccount...
+var RBACIdentityType = promauto.NewCounterVec(rbacIdentityTypeOps, []string{"type"})
+
+// RBACServiceAccountRejected shows number of SAs that were rejected due to
+// their ACL and our RBAC policies.
+var RBACServiceAccountRejected = promauto.NewCounter(rbacServiceAccountRejectedOps)
+
+func AddAPIMetricsWithNamespace(namespace string) {
+	metrics.AddAPIMetricsWithNamespace(namespace)
+
+	rbacIdentityTypeOps.Namespace = namespace
+	RBACIdentityType = promauto.NewCounterVec(rbacIdentityTypeOps, []string{"type"})
+
+	rbacServiceAccountRejectedOps.Namespace = namespace
+	RBACServiceAccountRejected = promauto.NewCounter(rbacServiceAccountRejectedOps)
+}

server/acks_handlers.go

@@ -403,7 +403,6 @@ func generateRuleAckMap(acks []types.SystemWideRuleDisable) (ruleAcksMap map[typ
 		} else {
 			log.Error().Err(err).Interface(ruleIDStr, ack.RuleID).
 				Interface(errorKeyStr, ack.ErrorKey).Msg(compositeRuleIDError)
-
 		}
 	}
 	return

server/auth_middleware.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/RedHatInsights/insights-operator-utils/collections"
 	"github.com/RedHatInsights/insights-results-smart-proxy/auth"
+	"github.com/RedHatInsights/insights-results-smart-proxy/metrics"
 	types "github.com/RedHatInsights/insights-results-types"
 	"github.com/gorilla/mux"
 	"github.com/rs/zerolog/log"
@@ -119,7 +120,6 @@ func (server *HTTPServer) Authentication(next http.Handler, noAuthURLs []string)
 // Authorization middleware for checking permissions
 func (server *HTTPServer) Authorization(next http.Handler, noAuthURLs []string) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-
 		// for specific URLs it is ok to not use auth. mechanisms at all
 		// this is specific to OpenAPI JSON response and for all OPTION HTTP methods
 		if collections.StringInSlice(r.RequestURI, noAuthURLs) || r.Method == "OPTIONS" {
@@ -133,6 +133,8 @@ func (server *HTTPServer) Authorization(next http.Handler, noAuthURLs []string)
 			return
 		}
 
+		metrics.RBACIdentityType.WithLabelValues(token.Identity.Type).Inc()
+
 		// For now we will only log authorization and only handle service accounts. This logic should
 		// be for all users, but let's first make sure we won't disturb existing users by only
 		// logging unauthorized service accounts
@@ -141,6 +143,7 @@ func (server *HTTPServer) Authorization(next http.Handler, noAuthURLs []string)
 			// Check permissions for service accounts
 			if !server.rbacClient.IsAuthorized(auth.GetAuthTokenHeader(r)) {
 				log.Warn().Str(accountType, token.Identity.Type).Msg(accountNotAuthorized)
+				metrics.RBACServiceAccountRejected.Inc()
 				if server.rbacClient.IsEnforcing() {
 					handleServerError(w, &auth.AuthorizationError{ErrString: accountNotAuthorized})
 					return

server/auth_middleware_test.go

@@ -24,9 +24,12 @@ import (
 
 	"github.com/RedHatInsights/insights-results-smart-proxy/auth"
 	"github.com/RedHatInsights/insights-results-smart-proxy/tests/helpers"
+	"github.com/prometheus/client_golang/prometheus"
 
+	"github.com/RedHatInsights/insights-results-smart-proxy/metrics"
 	"github.com/RedHatInsights/insights-results-smart-proxy/server"
 	types "github.com/RedHatInsights/insights-results-types"
+	prommodels "github.com/prometheus/client_model/go"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -131,7 +134,6 @@ func TestGetCurrentOrgID(t *testing.T) {
 	testServer := server.HTTPServer{}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-
 			req := getRequest(t, tc.identity)
 
 			orgID, err := testServer.GetCurrentOrgID(req)
@@ -239,11 +241,6 @@ func (m *MockRBACClient) IsEnforcing() bool {
 }
 
 func TestAuthorizationMiddleware(t *testing.T) {
-	jsonData, err := json.Marshal(validIdentityXRH)
-	assert.NoError(t, err)
-
-	token := base64.StdEncoding.EncodeToString(jsonData)
-
 	testCases := []struct {
 		name           string
 		xrhHeader      types.Token
@@ -290,12 +287,17 @@ func TestAuthorizationMiddleware(t *testing.T) {
 					Type: "UnknownType",
 				}},
 			rbacClient:     &MockRBACClient{authorized: false, enforcing: true},
-			expectedStatus: http.StatusForbidden,
+			expectedStatus: http.StatusOK, // RBAC is only enforced on ServiceAccounts
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
+			jsonData, err := json.Marshal(tc.xrhHeader)
+			assert.NoError(t, err)
+
+			token := base64.StdEncoding.EncodeToString(jsonData)
+
 			req := httptest.NewRequest(http.MethodGet, "/", nil)
 			// Set the authorization header, anything but an empty string is enough for these UTs
 			req.Header.Set("x-rh-identity", token)
@@ -317,11 +319,20 @@ func TestAuthorizationMiddleware(t *testing.T) {
 				w.WriteHeader(http.StatusOK)
 			})
 
+			initValueRBACIdentityType := int64(getCounterVecValue(t, metrics.RBACIdentityType, tc.xrhHeader.Identity.Type))
+			initValueRBACServiceAccountRejected := int64(getCounterValue(t, metrics.RBACServiceAccountRejected))
 			authHandler := testServer.Authorization(handler, nil)
 
 			authHandler.ServeHTTP(recorder, req)
 
 			assert.Equal(t, tc.expectedStatus, recorder.Code)
+
+			assertCounterVecValue(t, 1, metrics.RBACIdentityType, tc.xrhHeader.Identity.Type, initValueRBACIdentityType)
+			if tc.expectedStatus == http.StatusForbidden {
+				assertCounterValue(t, 1, metrics.RBACServiceAccountRejected, initValueRBACServiceAccountRejected)
+			} else {
+				assertCounterValue(t, 0, metrics.RBACServiceAccountRejected, initValueRBACServiceAccountRejected)
+			}
 		})
 	}
 }
@@ -353,3 +364,29 @@ func TestAuthorization_NoAuthURLs(t *testing.T) {
 	// Verify that the response is OK and RBAC was not used
 	assert.Equal(t, http.StatusOK, rr.Code)
 }
+
+func assertCounterVecValue(tb testing.TB, expected int64, counterVec *prometheus.CounterVec, label string, initValue int64) {
+	assert.Equal(tb, float64(initValue+expected), getCounterVecValue(tb, counterVec, label))
+}
+
+func assertCounterValue(tb testing.TB, expected int64, counter prometheus.Counter, initValue int64) {
+	assert.Equal(tb, float64(expected+initValue), getCounterValue(tb, counter))
+}
+
+func getCounterVecValue(tb testing.TB, counterVec *prometheus.CounterVec, label string) float64 {
+	counter, err := counterVec.GetMetricWithLabelValues(label)
+	if err != nil {
+		tb.Errorf("Unable to get counter from counterVec %v", err)
+	}
+	return getCounterValue(tb, counter)
+}
+
+func getCounterValue(tb testing.TB, counter prometheus.Counter) float64 {
+	pb := &prommodels.Metric{}
+	err := counter.Write(pb)
+	if err != nil {
+		tb.Errorf("Unable to get counter from counter %v", err)
+	}
+
+	return pb.GetCounter().GetValue()
+}

smart_proxy.go

@@ -30,12 +30,12 @@ import (
 
 	"github.com/RedHatInsights/insights-content-service/groups"
 	"github.com/RedHatInsights/insights-operator-utils/logger"
-	"github.com/RedHatInsights/insights-operator-utils/metrics"
 	"github.com/rs/zerolog/log"
 
 	"github.com/RedHatInsights/insights-results-smart-proxy/amsclient"
 	"github.com/RedHatInsights/insights-results-smart-proxy/auth"
 	"github.com/RedHatInsights/insights-results-smart-proxy/conf"
+	"github.com/RedHatInsights/insights-results-smart-proxy/metrics"
 	"github.com/RedHatInsights/insights-results-smart-proxy/server"
 	"github.com/RedHatInsights/insights-results-smart-proxy/services"