MGMT-18863: Fix bug in host event list endpoint

internal/events/event.go

@@ -355,32 +355,31 @@ func (e Events) prepareEventsTable(ctx context.Context, tx *gorm.DB, clusterID *
 	//host bound to a cluster or registered to a bound infra-env) check the access permission
 	//relative to the cluster ownership
 	if clusterBoundEvents() {
-		tx = tx.Model(&common.Cluster{}).
-			Select("events.*, clusters.user_name, clusters.org_id").
-			Joins("INNER JOIN events ON clusters.id = events.cluster_id")
+		tx = tx.Model(&common.Event{}).Select("events.*, clusters.user_name, clusters.org_id").
+			Joins("INNER JOIN clusters ON clusters.id = events.cluster_id")
 
 		// if deleted hosts flag is true, we need to add 'deleted_at' to know whether events are related to a deleted host
 		if swag.BoolValue(deletedHosts) {
 			tx = tx.Select("events.*, clusters.user_name, clusters.org_id, hosts.deleted_at").
-				Joins("LEFT JOIN hosts ON events.host_id = hosts.id")
+				Joins("LEFT JOIN hosts ON hosts.id = events.host_id")
 		}
 		return tx
 	}
 
 	//for unbound events that are searched with infra-env id (whether events on hosts or the
 	//infra-env level itself) check the access permission relative to the infra-env ownership
 	if nonBoundEvents() {
-		return tx.Model(&common.InfraEnv{}).
-			Select("events.*, infra_envs.user_name, infra_envs.org_id").
-			Joins("INNER JOIN events ON infra_envs.id = events.infra_env_id")
+		return tx.Model(&common.Event{}).Select("events.*, infra_envs.user_name, infra_envs.org_id").
+			Joins("INNER JOIN infra_envs ON infra_envs.id = events.infra_env_id")
 	}
 
-	//for query made on the host only check the permission relative to it's infra-env. since
-	//host table does not contain an org_id we can not perform a join on that table and has to go
-	//through the infra-env table which is good because authorization is done on the infra-level
+	// Events must be linked to the infra_envs table and then to the hosts table
+	// The hosts table does not hold an org_id, so permissions related fields must be supplied by the infra_env
 	if hostOnlyEvents() {
-		return tx.Model(&common.Host{}).Select("events.*, infra_envs.user_name, infra_envs.org_id").
-			Joins("INNER JOIN infra_envs ON hosts.infra_env_id = infra_envs.id").Joins("INNER JOIN events ON events.host_id = hosts.id")
+		return tx.Model(&common.Event{}).Select("events.*, infra_envs.user_name, infra_envs.org_id").
+			Joins("INNER JOIN infra_envs ON infra_envs.id = events.infra_env_id").
+			Joins("INNER JOIN hosts ON hosts.id = events.host_id"). // This join is here to ensure that only events for a host that exists are fetched
+			Where("hosts.deleted_at IS NULL")                       // Only interested in active hosts
 	}
 
 	//non supported option

internal/events/event_test.go

@@ -3,6 +3,7 @@ package events
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"testing"
 	"time"
@@ -177,6 +178,8 @@ var _ = Describe("Events library", func() {
 		infraEnv2 = strfmt.UUID("705c994b-eaa0-4b77-880b-66d4cd34cb4e")
 		host      = strfmt.UUID("1e45d128-4a69-4e71-9b50-a0c627217f3e")
 		host2     = strfmt.UUID("ad647798-a7af-4b1d-b96e-69f1beb9b4c3")
+		i1        common.InfraEnv
+		i2        common.InfraEnv
 	)
 	BeforeEach(func() {
 		db, dbName = common.PrepareTestDB()
@@ -186,9 +189,9 @@ var _ = Describe("Events library", func() {
 		Expect(db.Create(&c1).Error).ShouldNot(HaveOccurred())
 		c2 := common.Cluster{Cluster: models.Cluster{ID: &cluster2, OpenshiftClusterID: strfmt.UUID(uuid.New().String()), UserName: "user2", OrgID: "org1"}}
 		Expect(db.Create(&c2).Error).ShouldNot(HaveOccurred())
-		i1 := common.InfraEnv{InfraEnv: models.InfraEnv{ID: &infraEnv1, UserName: "user1", OrgID: "org1"}}
+		i1 = common.InfraEnv{InfraEnv: models.InfraEnv{ID: &infraEnv1, UserName: "user1", OrgID: "org1"}}
 		Expect(db.Create(&i1).Error).ShouldNot(HaveOccurred())
-		i2 := common.InfraEnv{InfraEnv: models.InfraEnv{ID: &infraEnv2, UserName: "user2", OrgID: "org1"}}
+		i2 = common.InfraEnv{InfraEnv: models.InfraEnv{ID: &infraEnv2, UserName: "user2", OrgID: "org1"}}
 		Expect(db.Create(&i2).Error).ShouldNot(HaveOccurred())
 	})
 	numOfEventsRetrieved := func(clusterID *strfmt.UUID, HostIds []strfmt.UUID, infraEnvID *strfmt.UUID) int {
@@ -273,6 +276,35 @@ var _ = Describe("Events library", func() {
 
 			Expect(numOfEventsRetrieved(&cluster2, nil, nil)).Should(Equal(0))
 		})
+
+		It("Should only list host specific events if infraenv for that host is present", func() {
+			// Create a host and link it to an infraEnv
+			h := models.Host{InfraEnvID: infraEnv1, ID: &host}
+			Expect(db.Create(&h).Error).ShouldNot(HaveOccurred())
+
+			// Generate some events for this host and infraenv combination
+			for i := 1; i <= 10; i++ {
+				theEvents.V2AddEvent(context.TODO(), nil, &host, &infraEnv1,
+					eventgen.HostRegistrationFailedEventName, models.EventSeverityInfo, fmt.Sprintf("Registration failed event %d:", i), time.Now())
+			}
+			Expect(numOfEventsRetrieved(nil, []strfmt.UUID{host}, nil)).Should(Equal(10))
+
+			// Delete the infraenv
+			Expect(db.Delete(&i1).Error).ShouldNot(HaveOccurred())
+			// Delete the host
+			Expect(db.Delete(&h).Error).ShouldNot(HaveOccurred())
+			// Recreate the host with a different infraenv
+			h = models.Host{InfraEnvID: infraEnv2, ID: &host}
+			Expect(db.Create(&h).Error).ShouldNot(HaveOccurred())
+
+			// Generate some events for the same host under the new infraenv
+			for i := 1; i <= 9; i++ {
+				theEvents.V2AddEvent(context.TODO(), nil, &host, &infraEnv2,
+					eventgen.HostRegistrationFailedEventName, models.EventSeverityInfo, fmt.Sprintf("Registration failed event %d:", i), time.Now())
+			}
+			// We should expect only the events for the new infraenv under the same host id
+			Expect(numOfEventsRetrieved(nil, []strfmt.UUID{host}, nil)).Should(Equal(9))
+		})
 	})
 
 	Context("events with request ID", func() {