Merge pull request #1664 from Bee-lee/extend-logs

 clean up rest of aggregator code, refactor auth mechanism

aggregator_test.go

@@ -94,48 +94,6 @@ func TestStartService(t *testing.T) {
 	*main.AutoMigratePtr = false
 }
 
-// TODO: fix with new groups consumer
-// func TestStartServiceWithMockBroker(t *testing.T) {
-//	const topicName = "topic"
-//	*main.AutoMigratePtr = true
-//
-//	helpers.RunTestWithTimeout(t, func(t *testing.T) {
-//		mockBroker := sarama.NewMockBroker(t, 0)
-//		defer mockBroker.Close()
-//
-//		mockBroker.SetHandlerByMap(helpers.GetHandlersMapForMockConsumer(t, mockBroker, topicName))
-//
-//		setEnvSettings(t, map[string]string{
-//			"INSIGHTS_RESULTS_AGGREGATOR__BROKER__ADDRESS": mockBroker.Addr(),
-//			"INSIGHTS_RESULTS_AGGREGATOR__BROKER__TOPIC":   topicName,
-//			"INSIGHTS_RESULTS_AGGREGATOR__BROKER__ENABLED": "true",
-//
-//			"INSIGHTS_RESULTS_AGGREGATOR__SERVER__ADDRESS":       ":8080",
-//			"INSIGHTS_RESULTS_AGGREGATOR__SERVER__API_PREFIX":    "/api/v1/",
-//			"INSIGHTS_RESULTS_AGGREGATOR__SERVER__API_SPEC_FILE": "openapi.json",
-//			"INSIGHTS_RESULTS_AGGREGATOR__SERVER__DEBUG":         "true",
-//
-//			"INSIGHTS_RESULTS_AGGREGATOR__STORAGE__DB_DRIVER":         "sqlite3",
-//			"INSIGHTS_RESULTS_AGGREGATOR__STORAGE__SQLITE_DATASOURCE": ":memory:",
-//
-//			"INSIGHTS_RESULTS_AGGREGATOR__CONTENT__PATH": "./tests/content/ok/",
-//		})
-//
-//		go func() {
-//			exitCode := main.StartService()
-//			if exitCode != main.ExitStatusOK {
-//				panic(fmt.Errorf("StartService exited with a code %v", exitCode))
-//			}
-//		}()
-//
-//		main.WaitForServiceToStart()
-//		errCode := main.StopService()
-//		assert.Equal(t, main.ExitStatusOK, errCode)
-//	}, testsTimeout)
-//
-//	*main.AutoMigratePtr = false
-//}
-
 func TestStartService_DBError(t *testing.T) {
 	helpers.RunTestWithTimeout(t, func(t testing.TB) {
 		buf := new(bytes.Buffer)

check_coverage.sh

@@ -14,7 +14,7 @@
 # limitations under the License.
 
 
-THRESHOLD=${COV_THRESHOLD:=77}
+THRESHOLD=${COV_THRESHOLD:=78}
 
 RED_BG=$(tput setab 1)
 GREEN_BG=$(tput setab 2)

go.mod

@@ -6,9 +6,9 @@ require (
 	github.com/BurntSushi/toml v0.3.1
 	github.com/DATA-DOG/go-sqlmock v1.4.1
 	github.com/RedHatInsights/insights-content-service v0.0.0-20201009081018-083923779f00
-	github.com/RedHatInsights/insights-operator-utils v1.23.8
+	github.com/RedHatInsights/insights-operator-utils v1.24.2
 	github.com/RedHatInsights/insights-results-aggregator-data v1.3.6
-	github.com/RedHatInsights/insights-results-types v1.3.18
+	github.com/RedHatInsights/insights-results-types v1.3.20
 	github.com/Shopify/sarama v1.27.1
 	github.com/deckarep/golang-set v1.7.1
 	github.com/gchaincl/sqlhooks v1.3.0

go.sum

@@ -43,8 +43,8 @@ github.com/RedHatInsights/insights-operator-utils v1.12.0/go.mod h1:mN5jURLpSG+j
 github.com/RedHatInsights/insights-operator-utils v1.21.0/go.mod h1:B2hizFGwXCc8MT34QqVJ1A8ANTyGQZQWXQw/gSCEsaU=
 github.com/RedHatInsights/insights-operator-utils v1.21.2/go.mod h1:3Pfsgsi7GCu2wgIqQlt1llpyQyyxsDWEGdgnPvadM40=
 github.com/RedHatInsights/insights-operator-utils v1.21.8/go.mod h1:qa1a8NdarIzcZkr5mu9fBw4OarOfg1qZFZC1vNGbyas=
-github.com/RedHatInsights/insights-operator-utils v1.23.8 h1:7JOQ+PMSjCsn95WYvIVQAJ49Rr53bsJQG2qW1vW1g2g=
-github.com/RedHatInsights/insights-operator-utils v1.23.8/go.mod h1:xsmvqwngwHqFvHPK+XARoZH8aR5Qcr13AGh4SJa3f40=
+github.com/RedHatInsights/insights-operator-utils v1.24.2 h1:+xUPgBGB/KGYqSUvsuVQ0PYMRBP5r5wh9RYp/ymaCNA=
+github.com/RedHatInsights/insights-operator-utils v1.24.2/go.mod h1:7qR/8rlMdiqoXAkZyQ5JhVrVNCa6SBwNUt4KMq/17j4=
 github.com/RedHatInsights/insights-results-aggregator v0.0.0-20200604090056-3534f6dd9c1c/go.mod h1:7Pc15NYXErx7BMJ4rF1Hacm+29G6atzjhwBpXNFMt+0=
 github.com/RedHatInsights/insights-results-aggregator-data v0.0.0-20200825113234-e84e924194bc/go.mod h1:DcDgoCCmBuUSKQOGrTi0BfFLdSjAp/KxIwyqKUd46sM=
 github.com/RedHatInsights/insights-results-aggregator-data v0.0.0-20201014142608-de97c4b07d5c/go.mod h1:x8IvreR2g24veCKVMXDPOR6a0D86QK9UCBfi5Xm5Gnc=
@@ -58,9 +58,8 @@ github.com/RedHatInsights/insights-results-aggregator-data v1.3.6 h1:RcZsn25t+km
 github.com/RedHatInsights/insights-results-aggregator-data v1.3.6/go.mod h1:tOwmlIB5irSv1mTLgONuLrsbTp4vokl4ClJFClrrXX0=
 github.com/RedHatInsights/insights-results-types v1.2.0/go.mod h1:6VVdMTGU/BAS2cW0KrHAUiDyocpyKqpPpEyp6AJ1tk8=
 github.com/RedHatInsights/insights-results-types v1.3.7/go.mod h1:6VVdMTGU/BAS2cW0KrHAUiDyocpyKqpPpEyp6AJ1tk8=
-github.com/RedHatInsights/insights-results-types v1.3.12/go.mod h1:6VVdMTGU/BAS2cW0KrHAUiDyocpyKqpPpEyp6AJ1tk8=
-github.com/RedHatInsights/insights-results-types v1.3.18 h1:V8wngjEc1j8r99P7e0EsCbh2FcsLXQkvDoWN3LJdfIY=
-github.com/RedHatInsights/insights-results-types v1.3.18/go.mod h1:6VVdMTGU/BAS2cW0KrHAUiDyocpyKqpPpEyp6AJ1tk8=
+github.com/RedHatInsights/insights-results-types v1.3.20 h1:02zlYmyZWkUCryHYgShrfYYO0ZJK3oYV5DaxeP5IZ2g=
+github.com/RedHatInsights/insights-results-types v1.3.20/go.mod h1:6VVdMTGU/BAS2cW0KrHAUiDyocpyKqpPpEyp6AJ1tk8=
 github.com/RedHatInsights/kafka-zerolog v0.0.0-20210304172207-928f026dc7ec/go.mod h1:HJul5oCsCRNiRlh/ayJDGdW3PzGlid/5aaQwJBn7was=
 github.com/RedHatInsights/kafka-zerolog v1.0.0 h1:4zPrLcwnfFl07qv9/ximlm1E/rWs93TkYnHrgNiU73A=
 github.com/RedHatInsights/kafka-zerolog v1.0.0/go.mod h1:HJul5oCsCRNiRlh/ayJDGdW3PzGlid/5aaQwJBn7was=

gocyclo.sh

@@ -20,4 +20,4 @@ then
     go get github.com/fzipp/gocyclo/cmd/gocyclo
 fi
 
-gocyclo -over 12 -avg .
+gocyclo -over 10 -avg .

migration/actual_migrations_test.go

@@ -1083,130 +1083,3 @@ func TestMigration28(t *testing.T) {
 	// not possible to insert more than 1 row per org
 	assert.Error(t, err)
 }
-
-// to be enabled after smart-proxy is merged
-/*
-func TestMigration29(t *testing.T) {
-	const selectUserIDQuery = "SELECT user_id FROM %v"
-	var tablesAffected = []string{
-		"advisor_ratings",
-		"cluster_rule_toggle",
-		"cluster_rule_user_feedback",
-		"cluster_user_rule_disable_feedback",
-		"rule_disable",
-	}
-
-	db, dbDriver, closer := prepareDBAndInfo(t)
-	defer closer()
-
-	if dbDriver == types.DBDriverSQLite3 {
-		// sqlite is no longer supported
-		return
-	}
-
-	err := migration.SetDBVersion(db, dbDriver, 28)
-	helpers.FailOnError(t, err)
-
-	// insert into report table because of DB constraints
-	_, err = db.Exec(`
-		INSERT INTO report (org_id, cluster, report, reported_at, last_checked_at)
-		VALUES ($1, $2, $3, $4, $5)
-		`,
-		testdata.OrgID,
-		testdata.ClusterName,
-		testdata.ClusterReport3Rules,
-		testdata.LastCheckedAt,
-		testdata.LastCheckedAt,
-	)
-	helpers.FailOnError(t, err)
-
-	// insert into advisor_ratings
-	_, err = db.Exec(
-		`INSERT INTO advisor_ratings
-			(org_id, user_id, rule_fqdn, error_key, rule_id)
-		VALUES
-			($1, $2, $3, $4, $5)`,
-		testdata.OrgID,
-		testdata.UserID,
-		testdata.Rule1CompositeID,
-		testdata.ErrorKey1,
-		testdata.Rule1ID,
-	)
-	helpers.FailOnError(t, err)
-
-	// insert into cluster_rule_toggle
-	_, err = db.Exec(
-		`INSERT INTO cluster_rule_toggle (cluster_id, rule_id, error_key, user_id, disabled, updated_at, org_id)
-				VALUES ($1, $2, $3, $4, $5, $6, $7)`,
-		testdata.ClusterName,
-		testdata.Rule1ID,
-		testdata.ErrorKey1,
-		testdata.UserID,
-		1,
-		testdata.LastCheckedAt,
-		testdata.OrgID,
-	)
-	helpers.FailOnError(t, err)
-
-	// insert into cluster_user_rule_disable_feedback
-	_, err = db.Exec(
-		`INSERT INTO cluster_user_rule_disable_feedback
-			(cluster_id, rule_id, error_key, user_id, message, added_at, updated_at, org_id)
-		VALUES
-			($1, $2, $3, $4, $5, $6, $7, $8)`,
-		testdata.ClusterName,
-		testdata.Rule1ID,
-		testdata.ErrorKey1,
-		testdata.UserID,
-		"",
-		testdata.LastCheckedAt,
-		testdata.LastCheckedAt,
-		testdata.OrgID,
-	)
-	helpers.FailOnError(t, err)
-
-	// insert into cluster_rule_user_feedback
-	_, err = db.Exec(
-		`INSERT INTO cluster_rule_user_feedback
-			(cluster_id, rule_id, error_key, user_id, message, added_at, updated_at, user_vote, org_id)
-		VALUES
-			($1, $2, $3, $4, $5, $6, $7, $8, $9)`,
-		testdata.ClusterName,
-		testdata.Rule1ID,
-		testdata.ErrorKey1,
-		testdata.UserID,
-		"",
-		testdata.LastCheckedAt,
-		testdata.LastCheckedAt,
-		1,
-		testdata.OrgID,
-	)
-	helpers.FailOnError(t, err)
-
-	// insert into rule_disable
-	_, err = db.Exec(
-		`INSERT INTO rule_disable
-			(org_id, user_id, rule_id, error_key, created_at)
-		VALUES
-			($1, $2, $3, $4, $5)`,
-		testdata.OrgID,
-		testdata.UserID,
-		testdata.Rule1ID,
-		testdata.ErrorKey1,
-		testdata.LastCheckedAt,
-	)
-	helpers.FailOnError(t, err)
-
-	err = migration.SetDBVersion(db, dbDriver, 29)
-	helpers.FailOnError(t, err)
-
-	for _, table := range tablesAffected {
-		var userID string
-		userIDQuery := fmt.Sprintf(selectUserIDQuery, table)
-		err = db.QueryRow(userIDQuery).Scan(&userID)
-		helpers.FailOnError(t, err)
-		// expect user_ids to be "0"
-		assert.Equal(t, userID, "0")
-	}
-}
-*/

server/auth.go

@@ -36,9 +36,6 @@ const (
 	malformedTokenMessage = "Malformed authentication token"
 )
 
-// Internal contains information about organization ID
-type Internal = types.Internal
-
 // Identity contains internal user info
 type Identity = types.Identity
 
@@ -78,8 +75,6 @@ func (server *HTTPServer) Authentication(next http.Handler, noAuthURLs []string)
 		}
 
 		tk := &types.Token{}
-		tkV2 := &types.TokenV2{}
-
 		// if we took JWT token, it has different structure than x-rh-identity
 		if server.Config.AuthType == "jwt" {
 			jwtPayload := &types.JWTPayload{}
@@ -93,26 +88,13 @@ func (server *HTTPServer) Authentication(next http.Handler, noAuthURLs []string)
 			// Map JWT token to inner token
 			tk.Identity = types.Identity{
 				AccountNumber: jwtPayload.AccountNumber,
-				Internal: types.Internal{
-					OrgID: jwtPayload.OrgID,
-				},
+				OrgID:         jwtPayload.OrgID,
 				User: types.User{
 					UserID: jwtPayload.UserID,
 				},
 			}
 		} else {
 			// auth type is xrh (x-rh-identity header)
-
-			// unmarshal new token structure (org_id on top level)
-			err = json.Unmarshal(decoded, tkV2)
-			if err != nil {
-				// malformed token, returns with HTTP code 403 as usual
-				log.Error().Err(err).Msg(malformedTokenMessage)
-				handleServerError(w, &UnauthorizedError{ErrString: malformedTokenMessage})
-				return
-			}
-
-			// unmarshal old token structure (org_id nested) too
 			err = json.Unmarshal(decoded, tk)
 			if err != nil {
 				// malformed token, returns with HTTP code 403 as usual
@@ -122,25 +104,20 @@ func (server *HTTPServer) Authentication(next http.Handler, noAuthURLs []string)
 			}
 		}
 
-		if tkV2.IdentityV2.OrgID != 0 {
-			log.Info().Msg("org_id found on top level in token structure (new format)")
-			// fill in old types.Token because many places in smart-proxy and aggregator rely on it.
-			tk.Identity.Internal.OrgID = tkV2.IdentityV2.OrgID
-		} else {
-			log.Error().Msg("org_id not found on top level in token structure (old format)")
-		}
-
-		if tk.Identity.AccountNumber == "" || tk.Identity.Internal.OrgID == 0 {
-			msg := fmt.Sprintf("error retrieving requester data from token. org_id [%v], account_number [%v], user data [%+v]",
-				tk.Identity.Internal.OrgID,
+		if tk.Identity.OrgID == 0 {
+			msg := fmt.Sprintf("error retrieving requester org_id from token. account_number [%v], user data [%+v]",
 				tk.Identity.AccountNumber,
-				tkV2.IdentityV2.User,
+				tk.Identity.User,
 			)
 			log.Error().Msg(msg)
 			handleServerError(w, &UnauthorizedError{ErrString: msg})
 			return
 		}
 
+		if tk.Identity.User.UserID == "" {
+			tk.Identity.User.UserID = "0"
+		}
+
 		// Everything went well, proceed with the request and set the
 		// caller to the user retrieved from the parsed token
 		ctx := context.WithValue(r.Context(), types.ContextKeyUser, tk.Identity)