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/bouncer.go

@@ -34,7 +34,7 @@ type Bouncer struct {
 	// Version holds the value of the "version" field.
 	Version string `json:"version"`
 	// LastPull holds the value of the "last_pull" field.
-	LastPull time.Time `json:"last_pull"`
+	LastPull *time.Time `json:"last_pull"`
 	// AuthType holds the value of the "auth_type" field.
 	AuthType     string `json:"auth_type"`
 	selectValues sql.SelectValues
@@ -126,7 +126,8 @@ func (b *Bouncer) assignValues(columns []string, values []any) error {
 			if value, ok := values[i].(*sql.NullTime); !ok {
 				return fmt.Errorf("unexpected type %T for field last_pull", values[i])
 			} else if value.Valid {
-				b.LastPull = value.Time
+				b.LastPull = new(time.Time)
+				*b.LastPull = value.Time
 			}
 		case bouncer.FieldAuthType:
 			if value, ok := values[i].(*sql.NullString); !ok {
@@ -193,8 +194,10 @@ func (b *Bouncer) String() string {
 	builder.WriteString("version=")
 	builder.WriteString(b.Version)
 	builder.WriteString(", ")
-	builder.WriteString("last_pull=")
-	builder.WriteString(b.LastPull.Format(time.ANSIC))
+	if v := b.LastPull; v != nil {
+		builder.WriteString("last_pull=")
+		builder.WriteString(v.Format(time.ANSIC))
+	}
 	builder.WriteString(", ")
 	builder.WriteString("auth_type=")
 	builder.WriteString(b.AuthType)

pkg/database/ent/bouncer/bouncer.go

@@ -71,8 +71,6 @@ var (
 	UpdateDefaultUpdatedAt func() time.Time
 	// DefaultIPAddress holds the default value on creation for the "ip_address" field.
 	DefaultIPAddress string
-	// DefaultLastPull holds the default value on creation for the "last_pull" field.
-	DefaultLastPull func() time.Time
 	// DefaultAuthType holds the default value on creation for the "auth_type" field.
 	DefaultAuthType string
 )

pkg/database/ent/bouncer/where.go

@@ -589,6 +589,16 @@ func LastPullLTE(v time.Time) predicate.Bouncer {
 	return predicate.Bouncer(sql.FieldLTE(FieldLastPull, v))
 }
 
+// LastPullIsNil applies the IsNil predicate on the "last_pull" field.
+func LastPullIsNil() predicate.Bouncer {
+	return predicate.Bouncer(sql.FieldIsNull(FieldLastPull))
+}
+
+// LastPullNotNil applies the NotNil predicate on the "last_pull" field.
+func LastPullNotNil() predicate.Bouncer {
+	return predicate.Bouncer(sql.FieldNotNull(FieldLastPull))
+}
+
 // AuthTypeEQ applies the EQ predicate on the "auth_type" field.
 func AuthTypeEQ(v string) predicate.Bouncer {
 	return predicate.Bouncer(sql.FieldEQ(FieldAuthType, v))

pkg/database/ent/bouncer_create.go

@@ -183,10 +183,6 @@ func (bc *BouncerCreate) defaults() {
 		v := bouncer.DefaultIPAddress
 		bc.mutation.SetIPAddress(v)
 	}
-	if _, ok := bc.mutation.LastPull(); !ok {
-		v := bouncer.DefaultLastPull()
-		bc.mutation.SetLastPull(v)
-	}
 	if _, ok := bc.mutation.AuthType(); !ok {
 		v := bouncer.DefaultAuthType
 		bc.mutation.SetAuthType(v)
@@ -210,9 +206,6 @@ func (bc *BouncerCreate) check() error {
 	if _, ok := bc.mutation.Revoked(); !ok {
 		return &ValidationError{Name: "revoked", err: errors.New(`ent: missing required field "Bouncer.revoked"`)}
 	}
-	if _, ok := bc.mutation.LastPull(); !ok {
-		return &ValidationError{Name: "last_pull", err: errors.New(`ent: missing required field "Bouncer.last_pull"`)}
-	}
 	if _, ok := bc.mutation.AuthType(); !ok {
 		return &ValidationError{Name: "auth_type", err: errors.New(`ent: missing required field "Bouncer.auth_type"`)}
 	}
@@ -276,7 +269,7 @@ func (bc *BouncerCreate) createSpec() (*Bouncer, *sqlgraph.CreateSpec) {
 	}
 	if value, ok := bc.mutation.LastPull(); ok {
 		_spec.SetField(bouncer.FieldLastPull, field.TypeTime, value)
-		_node.LastPull = value
+		_node.LastPull = &value
 	}
 	if value, ok := bc.mutation.AuthType(); ok {
 		_spec.SetField(bouncer.FieldAuthType, field.TypeString, value)

pkg/database/ent/bouncer_update.go

@@ -136,6 +136,12 @@ func (bu *BouncerUpdate) SetNillableLastPull(t *time.Time) *BouncerUpdate {
 	return bu
 }
 
+// ClearLastPull clears the value of the "last_pull" field.
+func (bu *BouncerUpdate) ClearLastPull() *BouncerUpdate {
+	bu.mutation.ClearLastPull()
+	return bu
+}
+
 // SetAuthType sets the "auth_type" field.
 func (bu *BouncerUpdate) SetAuthType(s string) *BouncerUpdate {
 	bu.mutation.SetAuthType(s)
@@ -230,6 +236,9 @@ func (bu *BouncerUpdate) sqlSave(ctx context.Context) (n int, err error) {
 	if value, ok := bu.mutation.LastPull(); ok {
 		_spec.SetField(bouncer.FieldLastPull, field.TypeTime, value)
 	}
+	if bu.mutation.LastPullCleared() {
+		_spec.ClearField(bouncer.FieldLastPull, field.TypeTime)
+	}
 	if value, ok := bu.mutation.AuthType(); ok {
 		_spec.SetField(bouncer.FieldAuthType, field.TypeString, value)
 	}
@@ -361,6 +370,12 @@ func (buo *BouncerUpdateOne) SetNillableLastPull(t *time.Time) *BouncerUpdateOne
 	return buo
 }
 
+// ClearLastPull clears the value of the "last_pull" field.
+func (buo *BouncerUpdateOne) ClearLastPull() *BouncerUpdateOne {
+	buo.mutation.ClearLastPull()
+	return buo
+}
+
 // SetAuthType sets the "auth_type" field.
 func (buo *BouncerUpdateOne) SetAuthType(s string) *BouncerUpdateOne {
 	buo.mutation.SetAuthType(s)
@@ -485,6 +500,9 @@ func (buo *BouncerUpdateOne) sqlSave(ctx context.Context) (_node *Bouncer, err e
 	if value, ok := buo.mutation.LastPull(); ok {
 		_spec.SetField(bouncer.FieldLastPull, field.TypeTime, value)
 	}
+	if buo.mutation.LastPullCleared() {
+		_spec.ClearField(bouncer.FieldLastPull, field.TypeTime)
+	}
 	if value, ok := buo.mutation.AuthType(); ok {
 		_spec.SetField(bouncer.FieldAuthType, field.TypeString, value)
 	}

pkg/database/ent/migrate/schema.go

@@ -68,7 +68,7 @@ var (
 		{Name: "ip_address", Type: field.TypeString, Nullable: true, Default: ""},
 		{Name: "type", Type: field.TypeString, Nullable: true},
 		{Name: "version", Type: field.TypeString, Nullable: true},
-		{Name: "last_pull", Type: field.TypeTime},
+		{Name: "last_pull", Type: field.TypeTime, Nullable: true},
 		{Name: "auth_type", Type: field.TypeString, Default: "api-key"},
 	}
 	// BouncersTable holds the schema information for the "bouncers" table.

pkg/database/ent/mutation.go

@@ -2840,7 +2840,7 @@ func (m *BouncerMutation) LastPull() (r time.Time, exists bool) {
 // OldLastPull returns the old "last_pull" field's value of the Bouncer entity.
 // If the Bouncer object wasn't provided to the builder, the object is fetched from the database.
 // An error is returned if the mutation operation is not UpdateOne, or the database query fails.
-func (m *BouncerMutation) OldLastPull(ctx context.Context) (v time.Time, err error) {
+func (m *BouncerMutation) OldLastPull(ctx context.Context) (v *time.Time, err error) {
 	if !m.op.Is(OpUpdateOne) {
 		return v, errors.New("OldLastPull is only allowed on UpdateOne operations")
 	}
@@ -2854,9 +2854,22 @@ func (m *BouncerMutation) OldLastPull(ctx context.Context) (v time.Time, err err
 	return oldValue.LastPull, nil
 }
 
+// ClearLastPull clears the value of the "last_pull" field.
+func (m *BouncerMutation) ClearLastPull() {
+	m.last_pull = nil
+	m.clearedFields[bouncer.FieldLastPull] = struct{}{}
+}
+
+// LastPullCleared returns if the "last_pull" field was cleared in this mutation.
+func (m *BouncerMutation) LastPullCleared() bool {
+	_, ok := m.clearedFields[bouncer.FieldLastPull]
+	return ok
+}
+
 // ResetLastPull resets all changes to the "last_pull" field.
 func (m *BouncerMutation) ResetLastPull() {
 	m.last_pull = nil
+	delete(m.clearedFields, bouncer.FieldLastPull)
 }
 
 // SetAuthType sets the "auth_type" field.
@@ -3135,6 +3148,9 @@ func (m *BouncerMutation) ClearedFields() []string {
 	if m.FieldCleared(bouncer.FieldVersion) {
 		fields = append(fields, bouncer.FieldVersion)
 	}
+	if m.FieldCleared(bouncer.FieldLastPull) {
+		fields = append(fields, bouncer.FieldLastPull)
+	}
 	return fields
 }
 
@@ -3158,6 +3174,9 @@ func (m *BouncerMutation) ClearField(name string) error {
 	case bouncer.FieldVersion:
 		m.ClearVersion()
 		return nil
+	case bouncer.FieldLastPull:
+		m.ClearLastPull()
+		return nil
 	}
 	return fmt.Errorf("unknown Bouncer nullable field %s", name)
 }

pkg/database/ent/runtime.go

@@ -72,10 +72,6 @@ func init() {
 	bouncerDescIPAddress := bouncerFields[5].Descriptor()
 	// bouncer.DefaultIPAddress holds the default value on creation for the ip_address field.
 	bouncer.DefaultIPAddress = bouncerDescIPAddress.Default.(string)
-	// bouncerDescLastPull is the schema descriptor for last_pull field.
-	bouncerDescLastPull := bouncerFields[8].Descriptor()
-	// bouncer.DefaultLastPull holds the default value on creation for the last_pull field.
-	bouncer.DefaultLastPull = bouncerDescLastPull.Default.(func() time.Time)
 	// bouncerDescAuthType is the schema descriptor for auth_type field.
 	bouncerDescAuthType := bouncerFields[9].Descriptor()
 	// bouncer.DefaultAuthType holds the default value on creation for the auth_type field.

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.'
 }
-