Merge pull request #10047 from hashicorp/dnephin/config-entry-validate

state: reduce arguments to validateProposedConfigEntryInServiceGraph

agent/consul/state/config_entry.go

@@ -174,8 +174,8 @@ func (s *Store) EnsureConfigEntry(idx uint64, conf structs.ConfigEntry) error {
 
 // ensureConfigEntryTxn upserts a config entry inside of a transaction.
 func ensureConfigEntryTxn(tx WriteTxn, idx uint64, conf structs.ConfigEntry) error {
-	// Check for existing configuration.
+	q := newConfigEntryQuery(conf)
-	existing, err := tx.First(tableConfigEntries, indexID, newConfigEntryQuery(conf))
+	existing, err := tx.First(tableConfigEntries, indexID, q)
 	if err != nil {
 		return fmt.Errorf("failed configuration lookup: %s", err)
 	}
@@ -196,7 +196,7 @@ func ensureConfigEntryTxn(tx WriteTxn, idx uint64, conf structs.ConfigEntry) err
 	}
 	raftIndex.ModifyIndex = idx
 
-	err = validateProposedConfigEntryInGraph(tx, conf.GetKind(), conf.GetName(), conf, conf.GetEnterpriseMeta())
+	err = validateProposedConfigEntryInGraph(tx, q, conf)
 	if err != nil {
 		return err // Err is already sufficiently decorated.
 	}
@@ -256,7 +256,8 @@ func (s *Store) DeleteConfigEntry(idx uint64, kind, name string, entMeta *struct
 
 // TODO: accept structs.ConfigEntry instead of individual fields
 func deleteConfigEntryTxn(tx WriteTxn, idx uint64, kind, name string, entMeta *structs.EnterpriseMeta) error {
-	existing, err := tx.First(tableConfigEntries, indexID, NewConfigEntryKindName(kind, name, entMeta))
+	q := NewConfigEntryKindName(kind, name, entMeta)
+	existing, err := tx.First(tableConfigEntries, indexID, q)
 	if err != nil {
 		return fmt.Errorf("failed config entry lookup: %s", err)
 	}
@@ -286,7 +287,7 @@ func deleteConfigEntryTxn(tx WriteTxn, idx uint64, kind, name string, entMeta *s
 		}
 	}
 
-	err = validateProposedConfigEntryInGraph(tx, kind, name, nil, entMeta)
+	err = validateProposedConfigEntryInGraph(tx, q, nil)
 	if err != nil {
 		return err // Err is already sufficiently decorated.
 	}
@@ -335,53 +336,46 @@ func insertConfigEntryWithTxn(tx WriteTxn, idx uint64, conf structs.ConfigEntry)
 // to the caller that they can correct.
 func validateProposedConfigEntryInGraph(
 	tx ReadTxn,
-	kind, name string,
+	kindName ConfigEntryKindName,
-	proposedEntry structs.ConfigEntry,
+	newEntry structs.ConfigEntry,
-	entMeta *structs.EnterpriseMeta,
 ) error {
-	validateAllChains := false
+	switch kindName.Kind {
-
-	switch kind {
 	case structs.ProxyDefaults:
-		if name != structs.ProxyConfigGlobal {
+		// TODO: why handle an invalid case?
+		if kindName.Name != structs.ProxyConfigGlobal {
 			return nil
 		}
-		validateAllChains = true
 	case structs.ServiceDefaults:
 	case structs.ServiceRouter:
 	case structs.ServiceSplitter:
 	case structs.ServiceResolver:
 	case structs.IngressGateway:
-		err := checkGatewayClash(tx, name, structs.IngressGateway, structs.TerminatingGateway, entMeta)
+		err := checkGatewayClash(tx, kindName, structs.TerminatingGateway)
 		if err != nil {
 			return err
 		}
 	case structs.TerminatingGateway:
-		err := checkGatewayClash(tx, name, structs.TerminatingGateway, structs.IngressGateway, entMeta)
+		err := checkGatewayClash(tx, kindName, structs.IngressGateway)
 		if err != nil {
 			return err
 		}
 	case structs.ServiceIntentions:
 	case structs.MeshConfig:
 	default:
-		return fmt.Errorf("unhandled kind %q during validation of %q", kind, name)
+		return fmt.Errorf("unhandled kind %q during validation of %q", kindName.Kind, kindName.Name)
 	}
 
-	return validateProposedConfigEntryInServiceGraph(tx, kind, name, proposedEntry, validateAllChains, entMeta)
+	return validateProposedConfigEntryInServiceGraph(tx, kindName, newEntry)
 }
 
-func checkGatewayClash(
+func checkGatewayClash(tx ReadTxn, kindName ConfigEntryKindName, otherKind string) error {
-	tx ReadTxn,
+	_, entry, err := configEntryTxn(tx, nil, otherKind, kindName.Name, &kindName.EnterpriseMeta)
-	name, selfKind, otherKind string,
-	entMeta *structs.EnterpriseMeta,
-) error {
-	_, entry, err := configEntryTxn(tx, nil, otherKind, name, entMeta)
 	if err != nil {
 		return err
 	}
 	if entry != nil {
 		return fmt.Errorf("cannot create a %q config entry with name %q, "+
-			"a %q config entry with that name already exists", selfKind, name, otherKind)
+			"a %q config entry with that name already exists", kindName.Kind, kindName.Name, otherKind)
 	}
 	return nil
 }
@@ -484,10 +478,8 @@ func (s *Store) discoveryChainSourcesTxn(tx ReadTxn, ws memdb.WatchSet, dc strin
 
 func validateProposedConfigEntryInServiceGraph(
 	tx ReadTxn,
-	kind, name string,
+	kindName ConfigEntryKindName,
-	proposedEntry structs.ConfigEntry,
+	newEntry structs.ConfigEntry,
-	validateAllChains bool,
-	entMeta *structs.EnterpriseMeta,
 ) error {
 	// Collect all of the chains that could be affected by this change
 	// including our own.
@@ -498,9 +490,8 @@ func validateProposedConfigEntryInServiceGraph(
 		enforceIngressProtocolsMatch bool
 	)
 
-	if validateAllChains {
+	switch kindName.Kind {
-		// Must be proxy-defaults/global.
+	case structs.ProxyDefaults:
-
 		// Check anything that has a discovery chain entry. In the future we could
 		// somehow omit the ones that have a default protocol configured.
 
@@ -538,31 +529,31 @@ func validateProposedConfigEntryInServiceGraph(
 			checkIntentions = append(checkIntentions, ixn)
 		}
 
-	} else if kind == structs.ServiceIntentions {
+	case structs.ServiceIntentions:
 		// Check that the protocols match.
 
 		// This is the case for deleting a config entry
-		if proposedEntry == nil {
+		if newEntry == nil {
 			return nil
 		}
 
-		ixn, ok := proposedEntry.(*structs.ServiceIntentionsConfigEntry)
+		ixn, ok := newEntry.(*structs.ServiceIntentionsConfigEntry)
 		if !ok {
-			return fmt.Errorf("type %T is not a service intentions config entry", proposedEntry)
+			return fmt.Errorf("type %T is not a service intentions config entry", newEntry)
 		}
 		checkIntentions = append(checkIntentions, ixn)
 
-	} else if kind == structs.IngressGateway {
+	case structs.IngressGateway:
 		// Checking an ingress pointing to multiple chains.
 
 		// This is the case for deleting a config entry
-		if proposedEntry == nil {
+		if newEntry == nil {
 			return nil
 		}
 
-		ingress, ok := proposedEntry.(*structs.IngressGatewayConfigEntry)
+		ingress, ok := newEntry.(*structs.IngressGatewayConfigEntry)
 		if !ok {
-			return fmt.Errorf("type %T is not an ingress gateway config entry", proposedEntry)
+			return fmt.Errorf("type %T is not an ingress gateway config entry", newEntry)
 		}
 		checkIngress = append(checkIngress, ingress)
 
@@ -570,12 +561,12 @@ func validateProposedConfigEntryInServiceGraph(
 		// validating the protocol equivalence.
 		enforceIngressProtocolsMatch = true
 
-	} else {
+	default:
 		// Must be a single chain.
 
 		// Check to see if we should ensure L7 intentions have an L7 protocol.
 		_, ixn, err := getServiceIntentionsConfigEntryTxn(
-			tx, nil, name, nil, entMeta,
+			tx, nil, kindName.Name, nil, &kindName.EnterpriseMeta,
 		)
 		if err != nil {
 			return err
@@ -595,7 +586,7 @@ func validateProposedConfigEntryInServiceGraph(
 			checkIntentions = append(checkIntentions, ixn)
 		}
 
-		sid := structs.NewServiceID(name, entMeta)
+		sid := structs.NewServiceID(kindName.Name, &kindName.EnterpriseMeta)
 		checkChains[sid] = struct{}{}
 
 		iter, err := tx.Get(tableConfigEntries, "link", sid)
@@ -631,7 +622,7 @@ func validateProposedConfigEntryInServiceGraph(
 	}
 
 	overrides := map[ConfigEntryKindName]structs.ConfigEntry{
-		NewConfigEntryKindName(kind, name, entMeta): proposedEntry,
+		kindName: newEntry,
 	}
 
 	var (