db: don't set bouncer last_pull until first connection (#3020)

* db: don't set bouncer last_pull until first connection

* cscli bouncers prune: query creation date if they never connected

cmd/crowdsec-cli/bouncers.go

@@ -116,7 +116,12 @@ func (cli *cliBouncers) list() error {
 				valid = "pending"
 			}
 
-			if err := csvwriter.Write([]string{b.Name, b.IPAddress, valid, b.LastPull.Format(time.RFC3339), b.Type, b.Version, b.AuthType}); err != nil {
+			lastPull := ""
+			if b.LastPull != nil {
+				lastPull = b.LastPull.Format(time.RFC3339)
+			}
+
+			if err := csvwriter.Write([]string{b.Name, b.IPAddress, valid, lastPull, b.Type, b.Version, b.AuthType}); err != nil {
 				return fmt.Errorf("failed to write raw: %w", err)
 			}
 		}
@@ -259,7 +264,7 @@ func (cli *cliBouncers) prune(duration time.Duration, force bool) error {
 		}
 	}
 
-	bouncers, err := cli.db.QueryBouncersLastPulltimeLT(time.Now().UTC().Add(-duration))
+	bouncers, err := cli.db.QueryBouncersInactiveSince(time.Now().UTC().Add(-duration))
 	if err != nil {
 		return fmt.Errorf("unable to query bouncers: %w", err)
 	}

cmd/crowdsec-cli/bouncers_table.go

@@ -21,7 +21,12 @@ func getBouncersTable(out io.Writer, bouncers []*ent.Bouncer) {
 			revoked = emoji.Prohibited
 		}
 
-		t.AddRow(b.Name, b.IPAddress, revoked, b.LastPull.Format(time.RFC3339), b.Type, b.Version, b.AuthType)
+		lastPull := ""
+		if b.LastPull != nil {
+			lastPull = b.LastPull.Format(time.RFC3339)
+		}
+
+		t.AddRow(b.Name, b.IPAddress, revoked, lastPull, b.Type, b.Version, b.AuthType)
 	}
 
 	t.Render()

pkg/apiserver/controllers/v1/decisions.go

@@ -72,7 +72,7 @@ func (c *Controller) GetDecision(gctx *gin.Context) {
 		return
 	}
 
-	if time.Now().UTC().Sub(bouncerInfo.LastPull) >= time.Minute {
+	if bouncerInfo.LastPull == nil || time.Now().UTC().Sub(*bouncerInfo.LastPull) >= time.Minute {
 		if err := c.DBClient.UpdateBouncerLastPull(time.Now().UTC(), bouncerInfo.ID); err != nil {
 			log.Errorf("failed to update bouncer last pull: %v", err)
 		}
@@ -186,7 +186,7 @@ func writeStartupDecisions(gctx *gin.Context, filters map[string][]string, dbFun
 	return nil
 }
 
-func writeDeltaDecisions(gctx *gin.Context, filters map[string][]string, lastPull time.Time, dbFunc func(time.Time, map[string][]string) ([]*ent.Decision, error)) error {
+func writeDeltaDecisions(gctx *gin.Context, filters map[string][]string, lastPull *time.Time, dbFunc func(*time.Time, map[string][]string) ([]*ent.Decision, error)) error {
 	//respBuffer := bytes.NewBuffer([]byte{})
 	limit := 30000 //FIXME : make it configurable
 	needComma := false
@@ -348,8 +348,13 @@ func (c *Controller) StreamDecisionNonChunked(gctx *gin.Context, bouncerInfo *en
 	//data = KeepLongestDecision(data)
 	ret["new"] = FormatDecisions(data)
 
+	since := time.Time{}
+	if bouncerInfo.LastPull != nil {
+		since = bouncerInfo.LastPull.Add(-2 * time.Second)
+	}
+
 	// getting expired decisions
-	data, err = c.DBClient.QueryExpiredDecisionsSinceWithFilters(bouncerInfo.LastPull.Add((-2 * time.Second)), filters) // do we want to give exactly lastPull time ?
+	data, err = c.DBClient.QueryExpiredDecisionsSinceWithFilters(&since, filters) // do we want to give exactly lastPull time ?
 	if err != nil {
 		log.Errorf("unable to query expired decision for '%s' : %v", bouncerInfo.Name, err)
 		gctx.JSON(http.StatusInternalServerError, gin.H{"message": err.Error()})

pkg/database/bouncers.go

@@ -115,6 +115,15 @@ func (c *Client) UpdateBouncerTypeAndVersion(bType string, version string, id in
 	return nil
 }
 
-func (c *Client) QueryBouncersLastPulltimeLT(t time.Time) ([]*ent.Bouncer, error) {
-	return c.Ent.Bouncer.Query().Where(bouncer.LastPullLT(t)).All(c.CTX)
+func (c *Client) QueryBouncersInactiveSince(t time.Time) ([]*ent.Bouncer, error) {
+	return c.Ent.Bouncer.Query().Where(
+		// poor man's coalesce
+		bouncer.Or(
+			bouncer.LastPullLT(t),
+			bouncer.And(
+				bouncer.LastPullIsNil(),
+				bouncer.CreatedAtLT(t),
+			),
+		),
+	).All(c.CTX)
 }

pkg/database/decisions.go

@@ -254,11 +254,15 @@ func longestDecisionForScopeTypeValue(s *sql.Selector) {
 	)
 }
 
-func (c *Client) QueryExpiredDecisionsSinceWithFilters(since time.Time, filters map[string][]string) ([]*ent.Decision, error) {
+func (c *Client) QueryExpiredDecisionsSinceWithFilters(since *time.Time, filters map[string][]string) ([]*ent.Decision, error) {
 	query := c.Ent.Decision.Query().Where(
 		decision.UntilLT(time.Now().UTC()),
-		decision.UntilGT(since),
 	)
+
+	if since != nil {
+		query = query.Where(decision.UntilGT(*since))
+	}
+
 	// Allow a bouncer to ask for non-deduplicated results
 	if v, ok := filters["dedup"]; !ok || v[0] != "false" {
 		query = query.Where(longestDecisionForScopeTypeValue)
@@ -281,12 +285,15 @@ func (c *Client) QueryExpiredDecisionsSinceWithFilters(since time.Time, filters
 	return data, nil
 }
 
-func (c *Client) QueryNewDecisionsSinceWithFilters(since time.Time, filters map[string][]string) ([]*ent.Decision, error) {
+func (c *Client) QueryNewDecisionsSinceWithFilters(since *time.Time, filters map[string][]string) ([]*ent.Decision, error) {
 	query := c.Ent.Decision.Query().Where(
-		decision.CreatedAtGT(since),
 		decision.UntilGT(time.Now().UTC()),
 	)
 
+	if since != nil {
+		query = query.Where(decision.CreatedAtGT(*since))
+	}
+
 	// Allow a bouncer to ask for non-deduplicated results
 	if v, ok := filters["dedup"]; !ok || v[0] != "false" {
 		query = query.Where(longestDecisionForScopeTypeValue)

pkg/database/ent/schema/bouncer.go

@@ -28,8 +28,7 @@ func (Bouncer) Fields() []ent.Field {
 		field.String("ip_address").Default("").Optional().StructTag(`json:"ip_address"`),
 		field.String("type").Optional().StructTag(`json:"type"`),
 		field.String("version").Optional().StructTag(`json:"version"`),
-		field.Time("last_pull").
-			Default(types.UtcNow).StructTag(`json:"last_pull"`),
+		field.Time("last_pull").Nillable().Optional().StructTag(`json:"last_pull"`),
 		field.String("auth_type").StructTag(`json:"auth_type"`).Default(types.ApiKeyAuthType),
 	}
 }

test/bats/10_bouncers.bats

@@ -39,7 +39,30 @@ teardown() {
     assert_output --partial "API key for 'ciTestBouncer':"
     rune -0 cscli bouncers delete ciTestBouncer
     rune -0 cscli bouncers list -o json
-    assert_output '[]'
+    assert_json '[]'
+}
+
+@test "cscli bouncers list" {
+    export API_KEY=bouncerkey
+    rune -0 cscli bouncers add ciTestBouncer --key "$API_KEY"
+
+    rune -0 cscli bouncers list -o json
+    rune -0 jq -c '.[] | [.ip_address,.last_pull,.name]' <(output)
+    assert_json '["",null,"ciTestBouncer"]'
+    rune -0 cscli bouncers list -o raw
+    assert_line 'name,ip,revoked,last_pull,type,version,auth_type'
+    assert_line 'ciTestBouncer,,validated,,,,api-key'
+    rune -0 cscli bouncers list -o human
+    assert_output --regexp 'ciTestBouncer.*api-key.*'
+
+    # the first connection sets last_pull and ip address
+    rune -0 lapi-get '/v1/decisions'
+    rune -0 cscli bouncers list -o json
+    rune -0 jq -r '.[] | .ip_address' <(output)
+    assert_output 127.0.0.1
+    rune -0 cscli bouncers list -o json
+    rune -0 jq -r '.[] | .last_pull' <(output)
+    refute_output null
 }
 
 @test "we can create a bouncer with a known key" {
@@ -83,4 +106,3 @@ teardown() {
     rune -0 cscli bouncers prune
     assert_output 'No bouncers to prune.'
 }
-