diff --git a/.changelog/9151.txt b/.changelog/9151.txt new file mode 100644 index 0000000000..d5fbce91b6 --- /dev/null +++ b/.changelog/9151.txt @@ -0,0 +1,3 @@ +```release-note:improvement +server: remove config entry CAS in legacy intention API bridge code +``` diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go index dc56faf946..c0db5431eb 100644 --- a/agent/consul/config_endpoint.go +++ b/agent/consul/config_endpoint.go @@ -19,10 +19,6 @@ type ConfigEntry struct { // Apply does an upsert of the given config entry. func (c *ConfigEntry) Apply(args *structs.ConfigEntryRequest, reply *bool) error { - return c.applyInternal(args, reply, nil) -} - -func (c *ConfigEntry) applyInternal(args *structs.ConfigEntryRequest, reply *bool, normalizeAndValidateFn func(structs.ConfigEntry) error) error { if err := c.srv.validateEnterpriseRequest(args.Entry.GetEnterpriseMeta(), true); err != nil { return err } @@ -47,17 +43,11 @@ func (c *ConfigEntry) applyInternal(args *structs.ConfigEntryRequest, reply *boo } // Normalize and validate the incoming config entry as if it came from a user. - if normalizeAndValidateFn == nil { - if err := args.Entry.Normalize(); err != nil { - return err - } - if err := args.Entry.Validate(); err != nil { - return err - } - } else { - if err := normalizeAndValidateFn(args.Entry); err != nil { - return err - } + if err := args.Entry.Normalize(); err != nil { + return err + } + if err := args.Entry.Validate(); err != nil { + return err } if authz != nil && !args.Entry.CanWrite(authz) { diff --git a/agent/consul/fsm/commands_oss.go b/agent/consul/fsm/commands_oss.go index 5a5a530c8d..a914009a4d 100644 --- a/agent/consul/fsm/commands_oss.go +++ b/agent/consul/fsm/commands_oss.go @@ -291,6 +291,11 @@ func (c *FSM) applyIntentionOperation(buf []byte, index uint64) interface{} { []metrics.Label{{Name: "op", Value: string(req.Op)}}) defer metrics.MeasureSinceWithLabels([]string{"fsm", "intention"}, time.Now(), []metrics.Label{{Name: "op", Value: string(req.Op)}}) + + if req.Mutation != nil { + return c.state.IntentionMutation(index, req.Op, req.Mutation) + } + switch req.Op { case structs.IntentionOpCreate, structs.IntentionOpUpdate: //nolint:staticcheck diff --git a/agent/consul/intention_endpoint.go b/agent/consul/intention_endpoint.go index 7908749170..3ff26c7222 100644 --- a/agent/consul/intention_endpoint.go +++ b/agent/consul/intention_endpoint.go @@ -21,22 +21,11 @@ var ( ErrIntentionNotFound = errors.New("Intention not found") ) -// NewIntentionEndpoint returns a new Intention endpoint. -func NewIntentionEndpoint(srv *Server, logger hclog.Logger) *Intention { - return &Intention{ - srv: srv, - logger: logger, - configEntryEndpoint: &ConfigEntry{srv}, - } -} - // Intention manages the Connect intentions. type Intention struct { // srv is a pointer back to the server. srv *Server logger hclog.Logger - - configEntryEndpoint *ConfigEntry } func (s *Intention) checkIntentionID(id string) (bool, error) { @@ -69,10 +58,7 @@ func (s *Intention) legacyUpgradeCheck() error { } // Apply creates or updates an intention in the data store. -func (s *Intention) Apply( - args *structs.IntentionRequest, - reply *string) error { - +func (s *Intention) Apply(args *structs.IntentionRequest, reply *string) error { // Ensure that all service-intentions config entry writes go to the primary // datacenter. These will then be replicated to all the other datacenters. args.Datacenter = s.srv.config.PrimaryDatacenter @@ -87,6 +73,10 @@ func (s *Intention) Apply( return err } + if args.Mutation != nil { + return fmt.Errorf("Mutation field is internal only and must not be set via RPC") + } + // Always set a non-nil intention to avoid nil-access below if args.Intention == nil { args.Intention = &structs.Intention{} @@ -105,27 +95,26 @@ func (s *Intention) Apply( } var ( - configOp structs.ConfigEntryOp - configEntry *structs.ServiceIntentionsConfigEntry + mut *structs.IntentionMutation legacyWrite bool ) switch args.Op { case structs.IntentionOpCreate: legacyWrite = true - configOp, configEntry, err = s.computeApplyChangesLegacyCreate(accessorID, authz, &entMeta, args) + mut, err = s.computeApplyChangesLegacyCreate(accessorID, authz, &entMeta, args) case structs.IntentionOpUpdate: legacyWrite = true - configOp, configEntry, err = s.computeApplyChangesLegacyUpdate(accessorID, authz, &entMeta, args) + mut, err = s.computeApplyChangesLegacyUpdate(accessorID, authz, &entMeta, args) case structs.IntentionOpUpsert: legacyWrite = false - configOp, configEntry, err = s.computeApplyChangesUpsert(&entMeta, args) + mut, err = s.computeApplyChangesUpsert(accessorID, authz, &entMeta, args) case structs.IntentionOpDelete: if args.Intention.ID == "" { legacyWrite = false - configOp, configEntry, err = s.computeApplyChangesDelete(&entMeta, args) + mut, err = s.computeApplyChangesDelete(accessorID, authz, &entMeta, args) } else { legacyWrite = true - configOp, configEntry, err = s.computeApplyChangesLegacyDelete(accessorID, authz, &entMeta, args) + mut, err = s.computeApplyChangesLegacyDelete(accessorID, authz, &entMeta, args) } case structs.IntentionOpDeleteAll: // This is an internal operation initiated by the leader and is not @@ -145,54 +134,16 @@ func (s *Intention) Apply( *reply = "" } - if configOp == "" { - return nil // no-op - } + // Switch to the config entry manipulating flavor: + args.Mutation = mut + args.Intention = nil - // Commit indirectly by invoking the other RPC handler directly. - - if configOp == structs.ConfigEntryDelete { - configReq := &structs.ConfigEntryRequest{ - Datacenter: args.Datacenter, - WriteRequest: args.WriteRequest, - Op: structs.ConfigEntryDelete, - Entry: configEntry, - } - - var ignored struct{} - return s.configEntryEndpoint.Delete(configReq, &ignored) - } - - if configOp != structs.ConfigEntryUpsertCAS { - return fmt.Errorf("Invalid Intention config entry operation: %v", configOp) - } - - configReq := &structs.ConfigEntryRequest{ - Datacenter: args.Datacenter, - WriteRequest: args.WriteRequest, - Op: structs.ConfigEntryUpsertCAS, - Entry: configEntry, - } - - var normalizeAndValidateFn func(raw structs.ConfigEntry) error - if legacyWrite { - normalizeAndValidateFn = func(raw structs.ConfigEntry) error { - entry := raw.(*structs.ServiceIntentionsConfigEntry) - if err := entry.LegacyNormalize(); err != nil { - return err - } - - return entry.LegacyValidate() - } - } - - var applied bool - if err = s.configEntryEndpoint.applyInternal(configReq, &applied, normalizeAndValidateFn); err != nil { + resp, err := s.srv.raftApply(structs.IntentionRequestType, args) + if err != nil { return err } - - if !applied { - return fmt.Errorf("config entry failed to persist due to CAS failure: kind=%q, name=%q", configEntry.Kind, configEntry.Name) + if respErr, ok := resp.(error); ok { + return respErr } return nil @@ -203,14 +154,11 @@ func (s *Intention) computeApplyChangesLegacyCreate( authz acl.Authorizer, entMeta *structs.EnterpriseMeta, args *structs.IntentionRequest, -) (structs.ConfigEntryOp, *structs.ServiceIntentionsConfigEntry, error) { +) (*structs.IntentionMutation, error) { // This variant is just for legacy UUID-based intentions. args.Intention.DefaultNamespaces(entMeta) - // Even though the eventual config entry RPC will do an authz check and - // validation, if we do them here too we can generate error messages that - // make more sense for legacy edits. if !args.Intention.CanWrite(authz) { sn := args.Intention.SourceServiceName() dn := args.Intention.DestinationServiceName() @@ -219,7 +167,7 @@ func (s *Intention) computeApplyChangesLegacyCreate( "source", sn.String(), "destination", dn.String(), "accessorID", accessorID) - return "", nil, acl.ErrPermissionDenied + return nil, acl.ErrPermissionDenied } // If no ID is provided, generate a new ID. This must be done prior to @@ -227,13 +175,13 @@ func (s *Intention) computeApplyChangesLegacyCreate( // the entry is in the log, the state update MUST be deterministic or // the followers will not converge. if args.Intention.ID != "" { - return "", nil, fmt.Errorf("ID must be empty when creating a new intention") + return nil, fmt.Errorf("ID must be empty when creating a new intention") } var err error args.Intention.ID, err = lib.GenerateUUID(s.checkIntentionID) if err != nil { - return "", nil, err + return nil, err } // Set the created at args.Intention.CreatedAt = time.Now().UTC() @@ -245,36 +193,30 @@ func (s *Intention) computeApplyChangesLegacyCreate( } if err := s.validateEnterpriseIntention(args.Intention); err != nil { - return "", nil, err + return nil, err } //nolint:staticcheck if err := args.Intention.Validate(); err != nil { - return "", nil, err + return nil, err } - _, configEntry, err := s.srv.fsm.State().ConfigEntry(nil, structs.ServiceIntentions, args.Intention.DestinationName, args.Intention.DestinationEnterpriseMeta()) - if err != nil { - return "", nil, fmt.Errorf("service-intentions config entry lookup failed: %v", err) - } - - if configEntry == nil { - return structs.ConfigEntryUpsertCAS, args.Intention.ToConfigEntry(true), nil - } - prevEntry := configEntry.(*structs.ServiceIntentionsConfigEntry) - - if err := checkLegacyIntentionApplyAllowed(prevEntry); err != nil { - return "", nil, err - } - - upsertEntry := prevEntry.Clone() - upsertEntry.Sources = append(upsertEntry.Sources, args.Intention.ToSourceIntention(true)) - // NOTE: if the append of this source causes a duplicate source name the // config entry validation will fail so we don't have to check that // explicitly here. - return structs.ConfigEntryUpsertCAS, upsertEntry, nil + mut := &structs.IntentionMutation{ + Destination: args.Intention.DestinationServiceName(), + Value: args.Intention.ToSourceIntention(true), + } + + // Set the created/updated times. If this is an update instead of an insert + // the UpdateOver() will fix it up appropriately. + now := time.Now().UTC() + mut.Value.LegacyCreateTime = timePointer(now) + mut.Value.LegacyUpdateTime = timePointer(now) + + return mut, nil } func (s *Intention) computeApplyChangesLegacyUpdate( @@ -282,28 +224,21 @@ func (s *Intention) computeApplyChangesLegacyUpdate( authz acl.Authorizer, entMeta *structs.EnterpriseMeta, args *structs.IntentionRequest, -) (structs.ConfigEntryOp, *structs.ServiceIntentionsConfigEntry, error) { +) (*structs.IntentionMutation, error) { // This variant is just for legacy UUID-based intentions. - _, prevEntry, ixn, err := s.srv.fsm.State().IntentionGet(nil, args.Intention.ID) + _, _, ixn, err := s.srv.fsm.State().IntentionGet(nil, args.Intention.ID) if err != nil { - return "", nil, fmt.Errorf("Intention lookup failed: %v", err) + return nil, fmt.Errorf("Intention lookup failed: %v", err) } - if ixn == nil || prevEntry == nil { - return "", nil, fmt.Errorf("Cannot modify non-existent intention: '%s'", args.Intention.ID) + if ixn == nil { + return nil, fmt.Errorf("Cannot modify non-existent intention: '%s'", args.Intention.ID) } - if err := checkLegacyIntentionApplyAllowed(prevEntry); err != nil { - return "", nil, err - } - - // Even though the eventual config entry RPC will do an authz check and - // validation, if we do them here too we can generate error messages that - // make more sense for legacy edits. if !ixn.CanWrite(authz) { // todo(kit) Migrate intention access denial logging over to audit logging when we implement it s.logger.Warn("Update operation on intention denied due to ACLs", "intention", args.Intention.ID, "accessorID", accessorID) - return "", nil, acl.ErrPermissionDenied + return nil, acl.ErrPermissionDenied } args.Intention.DefaultNamespaces(entMeta) @@ -311,98 +246,100 @@ func (s *Intention) computeApplyChangesLegacyUpdate( // Prior to v1.9.0 renames of the destination side of an intention were // allowed, but that behavior doesn't work anymore. if ixn.DestinationServiceName() != args.Intention.DestinationServiceName() { - return "", nil, fmt.Errorf("Cannot modify DestinationNS or DestinationName for an intention once it exists.") + return nil, fmt.Errorf("Cannot modify DestinationNS or DestinationName for an intention once it exists.") } - // We always update the updatedat field. - args.Intention.UpdatedAt = time.Now().UTC() - // Default source type if args.Intention.SourceType == "" { args.Intention.SourceType = structs.IntentionSourceConsul } if err := s.validateEnterpriseIntention(args.Intention); err != nil { - return "", nil, err + return nil, err } // Validate. We do not validate on delete since it is valid to only // send an ID in that case. //nolint:staticcheck if err := args.Intention.Validate(); err != nil { - return "", nil, err + return nil, err } - upsertEntry := prevEntry.Clone() - - foundMatch := upsertEntry.UpdateSourceByLegacyID( - args.Intention.ID, - args.Intention.ToSourceIntention(true), - ) - if !foundMatch { - return "", nil, fmt.Errorf("Cannot modify non-existent intention: '%s'", args.Intention.ID) + mut := &structs.IntentionMutation{ + ID: args.Intention.ID, + Value: args.Intention.ToSourceIntention(true), } - return structs.ConfigEntryUpsertCAS, upsertEntry, nil + // Set the created/updated times. If this is an update instead of an insert + // the UpdateOver() will fix it up appropriately. + now := time.Now().UTC() + mut.Value.LegacyCreateTime = timePointer(now) + mut.Value.LegacyUpdateTime = timePointer(now) + + return mut, nil } func (s *Intention) computeApplyChangesUpsert( + accessorID string, + authz acl.Authorizer, entMeta *structs.EnterpriseMeta, args *structs.IntentionRequest, -) (structs.ConfigEntryOp, *structs.ServiceIntentionsConfigEntry, error) { +) (*structs.IntentionMutation, error) { // This variant is just for config-entry based intentions. if args.Intention.ID != "" { // This is a new-style only endpoint - return "", nil, fmt.Errorf("ID must not be specified") + return nil, fmt.Errorf("ID must not be specified") } args.Intention.DefaultNamespaces(entMeta) - prevEntry, err := s.getServiceIntentionsConfigEntry(args.Intention.DestinationName, args.Intention.DestinationEnterpriseMeta()) - if err != nil { - return "", nil, err + if !args.Intention.CanWrite(authz) { + sn := args.Intention.SourceServiceName() + dn := args.Intention.DestinationServiceName() + // todo(kit) Migrate intention access denial logging over to audit logging when we implement it + s.logger.Warn("Intention upsert denied due to ACLs", + "source", sn.String(), + "destination", dn.String(), + "accessorID", accessorID) + return nil, acl.ErrPermissionDenied + } + + _, prevEntry, err := s.srv.fsm.State().ConfigEntry(nil, structs.ServiceIntentions, args.Intention.DestinationName, args.Intention.DestinationEnterpriseMeta()) + if err != nil { + return nil, fmt.Errorf("Intention lookup failed: %v", err) } - // TODO(intentions): have service-intentions validation functions - // return structured errors so that we can rewrite the field prefix - // here so that the validation errors are not misleading. if prevEntry == nil { // Meta is NOT permitted here, as it would need to be persisted on // the enclosing config entry. if len(args.Intention.Meta) > 0 { - return "", nil, fmt.Errorf("Meta must not be specified") + return nil, fmt.Errorf("Meta must not be specified") } + } else { + if len(args.Intention.Meta) > 0 { + // Meta is NOT permitted here, but there is one exception. If + // you are updating a previous record, but that record lives + // within a config entry that itself has Meta, then you may + // incidentally ship the Meta right back to consul. + // + // In that case if Meta is provided, it has to be a perfect + // match for what is already on the enclosing config entry so + // it's safe to discard. + if !equalStringMaps(prevEntry.GetMeta(), args.Intention.Meta) { + return nil, fmt.Errorf("Meta must not be specified, or should be unchanged during an update.") + } - upsertEntry := args.Intention.ToConfigEntry(false) - - return structs.ConfigEntryUpsertCAS, upsertEntry, nil + // Now it is safe to discard + args.Intention.Meta = nil + } } - upsertEntry := prevEntry.Clone() - - if len(args.Intention.Meta) > 0 { - // Meta is NOT permitted here, but there is one exception. If - // you are updating a previous record, but that record lives - // within a config entry that itself has Meta, then you may - // incidentally ship the Meta right back to consul. - // - // In that case if Meta is provided, it has to be a perfect - // match for what is already on the enclosing config entry so - // it's safe to discard. - if !equalStringMaps(upsertEntry.Meta, args.Intention.Meta) { - return "", nil, fmt.Errorf("Meta must not be specified, or should be unchanged during an update.") - } - - // Now it is safe to discard - args.Intention.Meta = nil - } - - sn := args.Intention.SourceServiceName() - - upsertEntry.UpsertSourceByName(sn, args.Intention.ToSourceIntention(false)) - - return structs.ConfigEntryUpsertCAS, upsertEntry, nil + return &structs.IntentionMutation{ + Destination: args.Intention.DestinationServiceName(), + Source: args.Intention.SourceServiceName(), + Value: args.Intention.ToSourceIntention(false), + }, nil } func (s *Intention) computeApplyChangesLegacyDelete( @@ -410,93 +347,58 @@ func (s *Intention) computeApplyChangesLegacyDelete( authz acl.Authorizer, entMeta *structs.EnterpriseMeta, args *structs.IntentionRequest, -) (structs.ConfigEntryOp, *structs.ServiceIntentionsConfigEntry, error) { - _, prevEntry, ixn, err := s.srv.fsm.State().IntentionGet(nil, args.Intention.ID) +) (*structs.IntentionMutation, error) { + _, _, ixn, err := s.srv.fsm.State().IntentionGet(nil, args.Intention.ID) if err != nil { - return "", nil, fmt.Errorf("Intention lookup failed: %v", err) + return nil, fmt.Errorf("Intention lookup failed: %v", err) } - if ixn == nil || prevEntry == nil { - return "", nil, fmt.Errorf("Cannot delete non-existent intention: '%s'", args.Intention.ID) + if ixn == nil { + return nil, fmt.Errorf("Cannot delete non-existent intention: '%s'", args.Intention.ID) } - if err := checkLegacyIntentionApplyAllowed(prevEntry); err != nil { - return "", nil, err - } - - // Even though the eventual config entry RPC will do an authz check and - // validation, if we do them here too we can generate error messages that - // make more sense for legacy edits. if !ixn.CanWrite(authz) { // todo(kit) Migrate intention access denial logging over to audit logging when we implement it s.logger.Warn("Deletion operation on intention denied due to ACLs", "intention", args.Intention.ID, "accessorID", accessorID) - return "", nil, acl.ErrPermissionDenied + return nil, acl.ErrPermissionDenied } - upsertEntry := prevEntry.Clone() - - deleted := upsertEntry.DeleteSourceByLegacyID(args.Intention.ID) - if !deleted { - return "", nil, fmt.Errorf("Cannot delete non-existent intention: '%s'", args.Intention.ID) - } - - if upsertEntry == nil || len(upsertEntry.Sources) == 0 { - return structs.ConfigEntryDelete, &structs.ServiceIntentionsConfigEntry{ - Kind: structs.ServiceIntentions, - Name: prevEntry.Name, - EnterpriseMeta: prevEntry.EnterpriseMeta, - }, nil - } - - return structs.ConfigEntryUpsertCAS, upsertEntry, nil + return &structs.IntentionMutation{ + ID: args.Intention.ID, + }, nil } func (s *Intention) computeApplyChangesDelete( + accessorID string, + authz acl.Authorizer, entMeta *structs.EnterpriseMeta, args *structs.IntentionRequest, -) (structs.ConfigEntryOp, *structs.ServiceIntentionsConfigEntry, error) { +) (*structs.IntentionMutation, error) { args.Intention.DefaultNamespaces(entMeta) - prevEntry, err := s.getServiceIntentionsConfigEntry(args.Intention.DestinationName, args.Intention.DestinationEnterpriseMeta()) + if !args.Intention.CanWrite(authz) { + sn := args.Intention.SourceServiceName() + dn := args.Intention.DestinationServiceName() + // todo(kit) Migrate intention access denial logging over to audit logging when we implement it + s.logger.Warn("Intention delete denied due to ACLs", + "source", sn.String(), + "destination", dn.String(), + "accessorID", accessorID) + return nil, acl.ErrPermissionDenied + } + + // Pre-flight to avoid pointless raft operations. + _, _, ixn, err := s.srv.fsm.State().IntentionGetExact(nil, args.Intention.ToExact()) if err != nil { - return "", nil, err + return nil, fmt.Errorf("Intention lookup failed: %v", err) + } + if ixn == nil { + return nil, nil } - if prevEntry == nil { - return "", nil, nil // no op means no-op - } - - // NOTE: validation errors may be misleading! - - upsertEntry := prevEntry.Clone() - - sn := args.Intention.SourceServiceName() - - deleted := upsertEntry.DeleteSourceByName(sn) - if !deleted { - return "", nil, nil // no op means no-op - } - - if upsertEntry == nil || len(upsertEntry.Sources) == 0 { - return structs.ConfigEntryDelete, &structs.ServiceIntentionsConfigEntry{ - Kind: structs.ServiceIntentions, - Name: prevEntry.Name, - EnterpriseMeta: prevEntry.EnterpriseMeta, - }, nil - } - - return structs.ConfigEntryUpsertCAS, upsertEntry, nil -} - -func checkLegacyIntentionApplyAllowed(prevEntry *structs.ServiceIntentionsConfigEntry) error { - if prevEntry == nil { - return nil - } - if prevEntry.LegacyIDFieldsAreAllSet() { - return nil - } - - sn := prevEntry.DestinationServiceName() - return fmt.Errorf("cannot use legacy intention API to edit intentions with a destination of %q after editing them via a service-intentions config entry", sn.String()) + return &structs.IntentionMutation{ + Destination: args.Intention.DestinationServiceName(), + Source: args.Intention.SourceServiceName(), + }, nil } // Get returns a single intention by ID. @@ -835,23 +737,6 @@ func (s *Intention) validateEnterpriseIntention(ixn *structs.Intention) error { return nil } -func (s *Intention) getServiceIntentionsConfigEntry(name string, entMeta *structs.EnterpriseMeta) (*structs.ServiceIntentionsConfigEntry, error) { - _, raw, err := s.srv.fsm.State().ConfigEntry(nil, structs.ServiceIntentions, name, entMeta) - if err != nil { - return nil, fmt.Errorf("Intention lookup failed: %v", err) - } - - if raw == nil { - return nil, nil - } - - configEntry, ok := raw.(*structs.ServiceIntentionsConfigEntry) - if !ok { - return nil, fmt.Errorf("invalid service config type %T", raw) - } - return configEntry, nil -} - func equalStringMaps(a, b map[string]string) bool { if len(a) != len(b) { return false diff --git a/agent/consul/server_register.go b/agent/consul/server_register.go index d1e44b5bc3..c0fda1b9bb 100644 --- a/agent/consul/server_register.go +++ b/agent/consul/server_register.go @@ -11,7 +11,7 @@ func init() { registerEndpoint(func(s *Server) interface{} { return &FederationState{s} }) registerEndpoint(func(s *Server) interface{} { return &DiscoveryChain{s} }) registerEndpoint(func(s *Server) interface{} { return &Health{s} }) - registerEndpoint(func(s *Server) interface{} { return NewIntentionEndpoint(s, s.loggers.Named(logging.Intentions)) }) + registerEndpoint(func(s *Server) interface{} { return &Intention{s, s.loggers.Named(logging.Intentions)} }) registerEndpoint(func(s *Server) interface{} { return &Internal{s, s.loggers.Named(logging.Internal)} }) registerEndpoint(func(s *Server) interface{} { return &KVS{s, s.loggers.Named(logging.KV)} }) registerEndpoint(func(s *Server) interface{} { return &Operator{s, s.loggers.Named(logging.Operator)} }) diff --git a/agent/consul/state/config_entry.go b/agent/consul/state/config_entry.go index 328ad15d33..8df8f18760 100644 --- a/agent/consul/state/config_entry.go +++ b/agent/consul/state/config_entry.go @@ -180,7 +180,7 @@ func (s *Store) EnsureConfigEntry(idx uint64, conf structs.ConfigEntry, entMeta } // ensureConfigEntryTxn upserts a config entry inside of a transaction. -func ensureConfigEntryTxn(tx *txn, idx uint64, conf structs.ConfigEntry, entMeta *structs.EnterpriseMeta) error { +func ensureConfigEntryTxn(tx WriteTxn, idx uint64, conf structs.ConfigEntry, entMeta *structs.EnterpriseMeta) error { // Check for existing configuration. existing, err := firstConfigEntryWithTxn(tx, conf.GetKind(), conf.GetName(), entMeta) if err != nil { @@ -254,6 +254,14 @@ func (s *Store) DeleteConfigEntry(idx uint64, kind, name string, entMeta *struct tx := s.db.WriteTxn(idx) defer tx.Abort() + if err := deleteConfigEntryTxn(tx, idx, kind, name, entMeta); err != nil { + return err + } + + return tx.Commit() +} + +func deleteConfigEntryTxn(tx WriteTxn, idx uint64, kind, name string, entMeta *structs.EnterpriseMeta) error { // Try to retrieve the existing config entry. existing, err := firstConfigEntryWithTxn(tx, kind, name, entMeta) if err != nil { @@ -298,10 +306,10 @@ func (s *Store) DeleteConfigEntry(idx uint64, kind, name string, entMeta *struct return fmt.Errorf("failed updating index: %s", err) } - return tx.Commit() + return nil } -func insertConfigEntryWithTxn(tx *txn, idx uint64, conf structs.ConfigEntry) error { +func insertConfigEntryWithTxn(tx WriteTxn, idx uint64, conf structs.ConfigEntry) error { if conf == nil { return fmt.Errorf("cannot insert nil config entry") } diff --git a/agent/consul/state/intention.go b/agent/consul/state/intention.go index 2dc0578234..f1f67866c8 100644 --- a/agent/consul/state/intention.go +++ b/agent/consul/state/intention.go @@ -207,6 +207,294 @@ func (s *Store) legacyIntentionsListTxn(tx ReadTxn, ws memdb.WatchSet, entMeta * var ErrLegacyIntentionsAreDisabled = errors.New("Legacy intention modifications are disabled after the config entry migration.") +func (s *Store) IntentionMutation(idx uint64, op structs.IntentionOp, mut *structs.IntentionMutation) error { + tx := s.db.WriteTxn(idx) + defer tx.Abort() + + usingConfigEntries, err := areIntentionsInConfigEntries(tx, nil) + if err != nil { + return err + } + if !usingConfigEntries { + return errors.New("state: IntentionMutation() is not allowed when intentions are not stored in config entries") + } + + switch op { + case structs.IntentionOpCreate: + if err := s.intentionMutationLegacyCreate(tx, idx, mut.Destination, mut.Value); err != nil { + return err + } + case structs.IntentionOpUpdate: + if err := s.intentionMutationLegacyUpdate(tx, idx, mut.ID, mut.Value); err != nil { + return err + } + case structs.IntentionOpDelete: + if mut.ID == "" { + if err := s.intentionMutationDelete(tx, idx, mut.Destination, mut.Source); err != nil { + return err + } + } else { + if err := s.intentionMutationLegacyDelete(tx, idx, mut.ID); err != nil { + return err + } + } + case structs.IntentionOpUpsert: + if err := s.intentionMutationUpsert(tx, idx, mut.Destination, mut.Source, mut.Value); err != nil { + return err + } + case structs.IntentionOpDeleteAll: + // This is an internal operation initiated by the leader and is not + // exposed for general RPC use. + return fmt.Errorf("Invalid Intention mutation operation '%s'", op) + default: + return fmt.Errorf("Invalid Intention mutation operation '%s'", op) + } + + return tx.Commit() +} + +func (s *Store) intentionMutationLegacyCreate( + tx WriteTxn, + idx uint64, + dest structs.ServiceName, + value *structs.SourceIntention, +) error { + _, configEntry, err := configEntryTxn(tx, nil, structs.ServiceIntentions, dest.Name, &dest.EnterpriseMeta) + if err != nil { + return fmt.Errorf("service-intentions config entry lookup failed: %v", err) + } + + var upsertEntry *structs.ServiceIntentionsConfigEntry + if configEntry == nil { + upsertEntry = &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: dest.Name, + EnterpriseMeta: dest.EnterpriseMeta, + Sources: []*structs.SourceIntention{value}, + } + } else { + prevEntry := configEntry.(*structs.ServiceIntentionsConfigEntry) + + if err := checkLegacyIntentionApplyAllowed(prevEntry); err != nil { + return err + } + + upsertEntry = prevEntry.Clone() + upsertEntry.Sources = append(upsertEntry.Sources, value) + } + + if err := upsertEntry.LegacyNormalize(); err != nil { + return err + } + if err := upsertEntry.LegacyValidate(); err != nil { + return err + } + + if err := ensureConfigEntryTxn(tx, idx, upsertEntry, upsertEntry.GetEnterpriseMeta()); err != nil { + return err + } + + return nil +} + +func (s *Store) intentionMutationLegacyUpdate( + tx WriteTxn, + idx uint64, + legacyID string, + value *structs.SourceIntention, +) error { + // This variant is just for legacy UUID-based intentions. + + _, prevEntry, ixn, err := s.IntentionGet(nil, legacyID) + if err != nil { + return fmt.Errorf("Intention lookup failed: %v", err) + } + if ixn == nil || prevEntry == nil { + return fmt.Errorf("Cannot modify non-existent intention: '%s'", legacyID) + } + + if err := checkLegacyIntentionApplyAllowed(prevEntry); err != nil { + return err + } + + upsertEntry := prevEntry.Clone() + + foundMatch := upsertEntry.UpdateSourceByLegacyID( + legacyID, + value, + ) + if !foundMatch { + return fmt.Errorf("Cannot modify non-existent intention: '%s'", legacyID) + } + + if err := upsertEntry.LegacyNormalize(); err != nil { + return err + } + if err := upsertEntry.LegacyValidate(); err != nil { + return err + } + + if err := ensureConfigEntryTxn(tx, idx, upsertEntry, upsertEntry.GetEnterpriseMeta()); err != nil { + return err + } + + return nil +} + +func (s *Store) intentionMutationDelete( + tx WriteTxn, + idx uint64, + dest structs.ServiceName, + src structs.ServiceName, +) error { + _, configEntry, err := configEntryTxn(tx, nil, structs.ServiceIntentions, dest.Name, &dest.EnterpriseMeta) + if err != nil { + return fmt.Errorf("service-intentions config entry lookup failed: %v", err) + } + if configEntry == nil { + return nil + } + + prevEntry := configEntry.(*structs.ServiceIntentionsConfigEntry) + upsertEntry := prevEntry.Clone() + + deleted := upsertEntry.DeleteSourceByName(src) + if !deleted { + return nil + } + + if upsertEntry == nil || len(upsertEntry.Sources) == 0 { + return deleteConfigEntryTxn( + tx, + idx, + structs.ServiceIntentions, + dest.Name, + &dest.EnterpriseMeta, + ) + } + + if err := upsertEntry.Normalize(); err != nil { + return err + } + if err := upsertEntry.Validate(); err != nil { + return err + } + + if err := ensureConfigEntryTxn(tx, idx, upsertEntry, upsertEntry.GetEnterpriseMeta()); err != nil { + return err + } + + return nil +} + +func (s *Store) intentionMutationLegacyDelete( + tx WriteTxn, + idx uint64, + legacyID string, +) error { + _, prevEntry, ixn, err := s.IntentionGet(nil, legacyID) + if err != nil { + return fmt.Errorf("Intention lookup failed: %v", err) + } + if ixn == nil || prevEntry == nil { + return fmt.Errorf("Cannot delete non-existent intention: '%s'", legacyID) + } + + if err := checkLegacyIntentionApplyAllowed(prevEntry); err != nil { + return err + } + + upsertEntry := prevEntry.Clone() + + deleted := upsertEntry.DeleteSourceByLegacyID(legacyID) + if !deleted { + return fmt.Errorf("Cannot delete non-existent intention: '%s'", legacyID) + } + + if upsertEntry == nil || len(upsertEntry.Sources) == 0 { + return deleteConfigEntryTxn( + tx, + idx, + structs.ServiceIntentions, + prevEntry.Name, + &prevEntry.EnterpriseMeta, + ) + } + + if err := upsertEntry.LegacyNormalize(); err != nil { + return err + } + if err := upsertEntry.LegacyValidate(); err != nil { + return err + } + + if err := ensureConfigEntryTxn(tx, idx, upsertEntry, upsertEntry.GetEnterpriseMeta()); err != nil { + return err + } + + return nil +} + +func (s *Store) intentionMutationUpsert( + tx WriteTxn, + idx uint64, + dest structs.ServiceName, + src structs.ServiceName, + value *structs.SourceIntention, +) error { + // This variant is just for config-entry based intentions. + + _, configEntry, err := configEntryTxn(tx, nil, structs.ServiceIntentions, dest.Name, &dest.EnterpriseMeta) + if err != nil { + return fmt.Errorf("service-intentions config entry lookup failed: %v", err) + } + + var prevEntry *structs.ServiceIntentionsConfigEntry + if configEntry != nil { + prevEntry = configEntry.(*structs.ServiceIntentionsConfigEntry) + } + + var upsertEntry *structs.ServiceIntentionsConfigEntry + + if prevEntry == nil { + upsertEntry = &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: dest.Name, + EnterpriseMeta: dest.EnterpriseMeta, + Sources: []*structs.SourceIntention{value}, + } + } else { + upsertEntry = prevEntry.Clone() + + upsertEntry.UpsertSourceByName(src, value) + } + + if err := upsertEntry.Normalize(); err != nil { + return err + } + if err := upsertEntry.Validate(); err != nil { + return err + } + + if err := ensureConfigEntryTxn(tx, idx, upsertEntry, upsertEntry.GetEnterpriseMeta()); err != nil { + return err + } + + return nil +} + +func checkLegacyIntentionApplyAllowed(prevEntry *structs.ServiceIntentionsConfigEntry) error { + if prevEntry == nil { + return nil + } + if prevEntry.LegacyIDFieldsAreAllSet() { + return nil + } + + sn := prevEntry.DestinationServiceName() + return fmt.Errorf("cannot use legacy intention API to edit intentions with a destination of %q after editing them via a service-intentions config entry", sn.String()) +} + // LegacyIntentionSet creates or updates an intention. // // Deprecated: Edit service-intentions config entries directly. diff --git a/agent/consul/state/intention_test.go b/agent/consul/state/intention_test.go index ba69773297..23b9caa759 100644 --- a/agent/consul/state/intention_test.go +++ b/agent/consul/state/intention_test.go @@ -13,6 +13,14 @@ import ( "github.com/stretchr/testify/require" ) +var ( + testLocation = time.FixedZone("UTC-8", -8*60*60) + + testTimeA = time.Date(1955, 11, 5, 6, 15, 0, 0, testLocation) + testTimeB = time.Date(1985, 10, 26, 1, 35, 0, 0, testLocation) + testTimeC = time.Date(2015, 10, 21, 16, 29, 0, 0, testLocation) +) + func testBothIntentionFormats(t *testing.T, f func(t *testing.T, s *Store, legacy bool)) { t.Helper() @@ -72,6 +80,8 @@ func TestStore_IntentionSetGet_basic(t *testing.T) { DestinationNS: "default", DestinationName: "web", Meta: map[string]string{}, + CreatedAt: testTimeA, + UpdatedAt: testTimeA, } // Inserting a with empty ID is disallowed. @@ -89,6 +99,8 @@ func TestStore_IntentionSetGet_basic(t *testing.T) { DestinationNS: "default", DestinationName: "web", Meta: map[string]string{}, + CreatedAt: testTimeA, + UpdatedAt: testTimeA, RaftIndex: structs.RaftIndex{ CreateIndex: lastIndex, ModifyIndex: lastIndex, @@ -103,10 +115,12 @@ func TestStore_IntentionSetGet_basic(t *testing.T) { Name: "web", Sources: []*structs.SourceIntention{ { - LegacyID: srcID, - Name: "*", - Action: structs.IntentionActionAllow, - LegacyMeta: map[string]string{}, + LegacyID: srcID, + Name: "*", + Action: structs.IntentionActionAllow, + LegacyMeta: map[string]string{}, + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, }, }, } @@ -258,6 +272,534 @@ func TestStore_LegacyIntentionDelete_failsAfterUpgrade(t *testing.T) { testutil.RequireErrorContains(t, err, ErrLegacyIntentionsAreDisabled.Error()) } +func TestStore_IntentionMutation(t *testing.T) { + testBothIntentionFormats(t, func(t *testing.T, s *Store, legacy bool) { + if legacy { + mut := &structs.IntentionMutation{} + err := s.IntentionMutation(1, structs.IntentionOpCreate, mut) + testutil.RequireErrorContains(t, err, "state: IntentionMutation() is not allowed when intentions are not stored in config entries") + } else { + testStore_IntentionMutation(t, s) + } + }) +} + +func testStore_IntentionMutation(t *testing.T, s *Store) { + lastIndex := uint64(1) + + defaultEntMeta := structs.DefaultEnterpriseMeta() + + var ( + id1 = testUUID() + id2 = testUUID() + id3 = testUUID() + ) + + eqEntry := func(t *testing.T, expect, got *structs.ServiceIntentionsConfigEntry) { + t.Helper() + + // Zero out some fields for comparison. + got = got.Clone() + got.RaftIndex = structs.RaftIndex{} + for _, src := range got.Sources { + src.LegacyCreateTime = nil + src.LegacyUpdateTime = nil + if len(src.LegacyMeta) == 0 { + src.LegacyMeta = nil + } + } + require.Equal(t, expect, got) + } + + // Try to create an intention without an ID to prove that LegacyValidate is being called. + testutil.RequireErrorContains(t, s.IntentionMutation(lastIndex, structs.IntentionOpCreate, &structs.IntentionMutation{ + Destination: structs.NewServiceName("api", defaultEntMeta), + Value: &structs.SourceIntention{ + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, + }, + }), `Sources[0].LegacyID must be set`) + + // Create intention and create config entry + { + require.NoError(t, s.IntentionMutation(lastIndex, structs.IntentionOpCreate, &structs.IntentionMutation{ + Destination: structs.NewServiceName("api", defaultEntMeta), + Value: &structs.SourceIntention{ + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + LegacyID: id1, + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, + }, + })) + + // Ensure it's there now. + idx, entry, ixn, err := s.IntentionGet(nil, id1) + require.NoError(t, err) + require.NotNil(t, entry) + require.NotNil(t, ixn) + require.Equal(t, lastIndex, idx) + + eqEntry(t, &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "api", + EnterpriseMeta: *defaultEntMeta, + Sources: []*structs.SourceIntention{ + { + LegacyID: id1, + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + Precedence: 9, + Type: structs.IntentionSourceConsul, + }, + }, + }, entry) + + // only one + _, entries, err := s.ConfigEntries(nil, nil) + require.NoError(t, err) + require.Len(t, entries, 1) + + lastIndex++ + } + + // Try to create a duplicate intention. + { + testutil.RequireErrorContains(t, s.IntentionMutation(lastIndex, structs.IntentionOpCreate, &structs.IntentionMutation{ + Destination: structs.NewServiceName("api", defaultEntMeta), + Value: &structs.SourceIntention{ + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionDeny, + LegacyID: id2, + LegacyCreateTime: &testTimeB, + LegacyUpdateTime: &testTimeB, + }, + }), `more than once`) + } + + // Create intention with existing config entry + { + require.NoError(t, s.IntentionMutation(lastIndex, structs.IntentionOpCreate, &structs.IntentionMutation{ + Destination: structs.NewServiceName("api", defaultEntMeta), + Value: &structs.SourceIntention{ + Name: "debug", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionDeny, + LegacyID: id2, + LegacyCreateTime: &testTimeB, + LegacyUpdateTime: &testTimeB, + }, + })) + + // Ensure it's there now. + idx, entry, ixn, err := s.IntentionGet(nil, id2) + require.NoError(t, err) + require.NotNil(t, entry) + require.NotNil(t, ixn) + require.Equal(t, lastIndex, idx) + + eqEntry(t, &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "api", + EnterpriseMeta: *defaultEntMeta, + Sources: []*structs.SourceIntention{ + { + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + LegacyID: id1, + Precedence: 9, + Type: structs.IntentionSourceConsul, + }, + { + Name: "debug", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionDeny, + LegacyID: id2, + Precedence: 9, + Type: structs.IntentionSourceConsul, + }, + }, + }, entry) + + // only one + _, entries, err := s.ConfigEntries(nil, nil) + require.NoError(t, err) + require.Len(t, entries, 1) + + lastIndex++ + } + + // Try to update an intention without specifying an ID + testutil.RequireErrorContains(t, s.IntentionMutation(lastIndex, structs.IntentionOpUpdate, &structs.IntentionMutation{ + ID: "", + Destination: structs.NewServiceName("api", defaultEntMeta), + Value: &structs.SourceIntention{ + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + }, + }), `failed config entry lookup: index error: UUID must be 36 characters`) + + // Try to update a non-existent intention + testutil.RequireErrorContains(t, s.IntentionMutation(lastIndex, structs.IntentionOpUpdate, &structs.IntentionMutation{ + ID: id3, + Destination: structs.NewServiceName("api", defaultEntMeta), + Value: &structs.SourceIntention{ + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + }, + }), `Cannot modify non-existent intention`) + + // Update an existing intention by ID + { + require.NoError(t, s.IntentionMutation(lastIndex, structs.IntentionOpUpdate, &structs.IntentionMutation{ + ID: id2, + Destination: structs.NewServiceName("api", defaultEntMeta), + Value: &structs.SourceIntention{ + Name: "debug", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionDeny, + LegacyID: id2, + LegacyCreateTime: &testTimeB, + LegacyUpdateTime: &testTimeC, + Description: "op update", + }, + })) + + // Ensure it's there now. + idx, entry, ixn, err := s.IntentionGet(nil, id2) + require.NoError(t, err) + require.NotNil(t, entry) + require.NotNil(t, ixn) + require.Equal(t, lastIndex, idx) + + eqEntry(t, &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "api", + EnterpriseMeta: *defaultEntMeta, + Sources: []*structs.SourceIntention{ + { + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + LegacyID: id1, + Precedence: 9, + Type: structs.IntentionSourceConsul, + }, + { + Name: "debug", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionDeny, + LegacyID: id2, + Precedence: 9, + Type: structs.IntentionSourceConsul, + Description: "op update", + }, + }, + }, entry) + + // only one + _, entries, err := s.ConfigEntries(nil, nil) + require.NoError(t, err) + require.Len(t, entries, 1) + + lastIndex++ + } + + // Try to delete a non-existent intention + testutil.RequireErrorContains(t, s.IntentionMutation(lastIndex, structs.IntentionOpDelete, &structs.IntentionMutation{ + ID: id3, + }), `Cannot delete non-existent intention`) + + // delete by id + { + require.NoError(t, s.IntentionMutation(lastIndex, structs.IntentionOpDelete, &structs.IntentionMutation{ + ID: id1, + })) + + // only one + _, entries, err := s.ConfigEntries(nil, nil) + require.NoError(t, err) + require.Len(t, entries, 1) + + eqEntry(t, &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "api", + EnterpriseMeta: *defaultEntMeta, + Sources: []*structs.SourceIntention{ + { + Name: "debug", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionDeny, + LegacyID: id2, + Precedence: 9, + Type: structs.IntentionSourceConsul, + Description: "op update", + }, + }, + }, entries[0].(*structs.ServiceIntentionsConfigEntry)) + + lastIndex++ + } + + // delete last one by id + { + require.NoError(t, s.IntentionMutation(lastIndex, structs.IntentionOpDelete, &structs.IntentionMutation{ + ID: id2, + })) + + // none one + _, entries, err := s.ConfigEntries(nil, nil) + require.NoError(t, err) + require.Empty(t, entries) + + lastIndex++ + } + + // upsert intention for first time + { + require.NoError(t, s.IntentionMutation(lastIndex, structs.IntentionOpUpsert, &structs.IntentionMutation{ + Destination: structs.NewServiceName("api", defaultEntMeta), + Value: &structs.SourceIntention{ + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + }, + })) + + // Ensure it's there now. + idx, entry, ixn, err := s.IntentionGetExact(nil, &structs.IntentionQueryExact{ + SourceNS: "default", + SourceName: "web", + DestinationNS: "default", + DestinationName: "api", + }) + require.NoError(t, err) + require.NotNil(t, entry) + require.NotNil(t, ixn) + require.Equal(t, lastIndex, idx) + + eqEntry(t, &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "api", + EnterpriseMeta: *defaultEntMeta, + Sources: []*structs.SourceIntention{ + { + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + Precedence: 9, + Type: structs.IntentionSourceConsul, + }, + }, + }, entry) + + // only one + _, entries, err := s.ConfigEntries(nil, nil) + require.NoError(t, err) + require.Len(t, entries, 1) + + lastIndex++ + } + + // upsert over itself (REPLACE) + { + require.NoError(t, s.IntentionMutation(lastIndex, structs.IntentionOpUpsert, &structs.IntentionMutation{ + Destination: structs.NewServiceName("api", defaultEntMeta), + Source: structs.NewServiceName("web", defaultEntMeta), + Value: &structs.SourceIntention{ + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + Description: "upserted over", + }, + })) + + // Ensure it's there now. + idx, entry, ixn, err := s.IntentionGetExact(nil, &structs.IntentionQueryExact{ + SourceNS: "default", + SourceName: "web", + DestinationNS: "default", + DestinationName: "api", + }) + require.NoError(t, err) + require.NotNil(t, entry) + require.NotNil(t, ixn) + require.Equal(t, lastIndex, idx) + require.Equal(t, "upserted over", ixn.Description) + + eqEntry(t, &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "api", + EnterpriseMeta: *defaultEntMeta, + Sources: []*structs.SourceIntention{ + { + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + Precedence: 9, + Type: structs.IntentionSourceConsul, + Description: "upserted over", + }, + }, + }, entry) + + // only one + _, entries, err := s.ConfigEntries(nil, nil) + require.NoError(t, err) + require.Len(t, entries, 1) + + lastIndex++ + } + + // upsert into existing config entry (APPEND) + { + require.NoError(t, s.IntentionMutation(lastIndex, structs.IntentionOpUpsert, &structs.IntentionMutation{ + Destination: structs.NewServiceName("api", defaultEntMeta), + Source: structs.NewServiceName("debug", defaultEntMeta), + Value: &structs.SourceIntention{ + Name: "debug", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionDeny, + }, + })) + + // Ensure it's there now. + idx, entry, ixn, err := s.IntentionGetExact(nil, &structs.IntentionQueryExact{ + SourceNS: "default", + SourceName: "debug", + DestinationNS: "default", + DestinationName: "api", + }) + require.NoError(t, err) + require.NotNil(t, entry) + require.NotNil(t, ixn) + require.Equal(t, lastIndex, idx) + + eqEntry(t, &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "api", + EnterpriseMeta: *defaultEntMeta, + Sources: []*structs.SourceIntention{ + { + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + Precedence: 9, + Type: structs.IntentionSourceConsul, + Description: "upserted over", + }, + { + Name: "debug", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionDeny, + Precedence: 9, + Type: structs.IntentionSourceConsul, + }, + }, + }, entry) + + // only one + _, entries, err := s.ConfigEntries(nil, nil) + require.NoError(t, err) + require.Len(t, entries, 1) + + lastIndex++ + } + + // Try to delete a non-existent intention by name + require.NoError(t, s.IntentionMutation(lastIndex, structs.IntentionOpDelete, &structs.IntentionMutation{ + Destination: structs.NewServiceName("api", defaultEntMeta), + Source: structs.NewServiceName("blurb", defaultEntMeta), + })) + + // delete by name + { + require.NoError(t, s.IntentionMutation(lastIndex, structs.IntentionOpDelete, &structs.IntentionMutation{ + Destination: structs.NewServiceName("api", defaultEntMeta), + Source: structs.NewServiceName("web", defaultEntMeta), + })) + + // only one + _, entries, err := s.ConfigEntries(nil, nil) + require.NoError(t, err) + require.Len(t, entries, 1) + + eqEntry(t, &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "api", + EnterpriseMeta: *defaultEntMeta, + Sources: []*structs.SourceIntention{ + { + Name: "debug", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionDeny, + Precedence: 9, + Type: structs.IntentionSourceConsul, + }, + }, + }, entries[0].(*structs.ServiceIntentionsConfigEntry)) + + lastIndex++ + } + + // delete last one by name + { + require.NoError(t, s.IntentionMutation(lastIndex, structs.IntentionOpDelete, &structs.IntentionMutation{ + Destination: structs.NewServiceName("api", defaultEntMeta), + Source: structs.NewServiceName("debug", defaultEntMeta), + })) + + // none one + _, entries, err := s.ConfigEntries(nil, nil) + require.NoError(t, err) + require.Empty(t, entries) + + lastIndex++ + } + + // Try to update an intention with an ID on a non-legacy config entry. + { + idFake := testUUID() + + require.NoError(t, s.EnsureConfigEntry(lastIndex, &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "new", + EnterpriseMeta: *defaultEntMeta, + Sources: []*structs.SourceIntention{ + { + Name: "web", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + Precedence: 9, + Type: structs.IntentionSourceConsul, + }, + }, + }, nil)) + + lastIndex++ + + // ...via create + testutil.RequireErrorContains(t, s.IntentionMutation(lastIndex, structs.IntentionOpCreate, &structs.IntentionMutation{ + Destination: structs.NewServiceName("new", defaultEntMeta), + Value: &structs.SourceIntention{ + Name: "old", + EnterpriseMeta: *defaultEntMeta, + Action: structs.IntentionActionAllow, + LegacyID: idFake, + }, + }), `cannot use legacy intention API to edit intentions with a destination`) + } +} + func TestStore_LegacyIntentionSet_emptyId(t *testing.T) { // note: irrelevant test for config entries variant s := testStateStore(t) @@ -282,24 +824,27 @@ func TestStore_IntentionSet_updateCreatedAt(t *testing.T) { testBothIntentionFormats(t, func(t *testing.T, s *Store, legacy bool) { // Build a valid intention var ( - id = testUUID() - createTime time.Time + id = testUUID() ) if legacy { ixn := structs.Intention{ - ID: id, - CreatedAt: time.Now().UTC(), + ID: id, + SourceNS: "default", + SourceName: "*", + DestinationNS: "default", + DestinationName: "web", + Action: structs.IntentionActionAllow, + CreatedAt: testTimeA, + UpdatedAt: testTimeA, } // Insert require.NoError(t, s.LegacyIntentionSet(1, &ixn)) - createTime = ixn.CreatedAt - // Change a value and test updating ixnUpdate := ixn - ixnUpdate.CreatedAt = createTime.Add(10 * time.Second) + ixnUpdate.CreatedAt = testTimeB require.NoError(t, s.LegacyIntentionSet(2, &ixnUpdate)) id = ixn.ID @@ -310,10 +855,12 @@ func TestStore_IntentionSet_updateCreatedAt(t *testing.T) { Name: "web", Sources: []*structs.SourceIntention{ { - LegacyID: id, - Name: "*", - Action: structs.IntentionActionAllow, - LegacyMeta: map[string]string{}, + LegacyID: id, + Name: "*", + Action: structs.IntentionActionAllow, + LegacyMeta: map[string]string{}, + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, }, }, } @@ -321,15 +868,13 @@ func TestStore_IntentionSet_updateCreatedAt(t *testing.T) { require.NoError(t, conf.LegacyNormalize()) require.NoError(t, conf.LegacyValidate()) require.NoError(t, s.EnsureConfigEntry(1, conf.Clone(), nil)) - - createTime = *conf.Sources[0].LegacyCreateTime } // Read it back and verify _, _, actual, err := s.IntentionGet(nil, id) require.NoError(t, err) require.NotNil(t, actual) - require.Equal(t, createTime, actual.CreatedAt) + require.Equal(t, testTimeA, actual.CreatedAt) }) } @@ -339,7 +884,14 @@ func TestStore_IntentionSet_metaNil(t *testing.T) { if legacy { // Build a valid intention ixn := &structs.Intention{ - ID: id, + ID: id, + SourceNS: "default", + SourceName: "*", + DestinationNS: "default", + DestinationName: "web", + Action: structs.IntentionActionAllow, + CreatedAt: testTimeA, + UpdatedAt: testTimeA, } // Insert @@ -351,9 +903,11 @@ func TestStore_IntentionSet_metaNil(t *testing.T) { Name: "web", Sources: []*structs.SourceIntention{ { - LegacyID: id, - Name: "*", - Action: structs.IntentionActionAllow, + LegacyID: id, + Name: "*", + Action: structs.IntentionActionAllow, + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, }, }, } @@ -380,8 +934,15 @@ func TestStore_IntentionSet_metaSet(t *testing.T) { if legacy { // Build a valid intention ixn := structs.Intention{ - ID: id, - Meta: expectMeta, + ID: id, + SourceNS: "default", + SourceName: "*", + DestinationNS: "default", + DestinationName: "web", + Action: structs.IntentionActionAllow, + CreatedAt: testTimeA, + UpdatedAt: testTimeA, + Meta: expectMeta, } // Insert @@ -394,10 +955,12 @@ func TestStore_IntentionSet_metaSet(t *testing.T) { Name: "web", Sources: []*structs.SourceIntention{ { - LegacyID: id, - Name: "*", - Action: structs.IntentionActionAllow, - LegacyMeta: expectMeta, + LegacyID: id, + Name: "*", + Action: structs.IntentionActionAllow, + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, + LegacyMeta: expectMeta, }, }, } @@ -428,7 +991,14 @@ func TestStore_IntentionDelete(t *testing.T) { // Create if legacy { ixn := &structs.Intention{ - ID: id, + ID: id, + SourceNS: "default", + SourceName: "*", + DestinationNS: "default", + DestinationName: "web", + Action: structs.IntentionActionAllow, + CreatedAt: testTimeA, + UpdatedAt: testTimeA, } lastIndex++ require.NoError(t, s.LegacyIntentionSet(lastIndex, ixn)) @@ -442,9 +1012,11 @@ func TestStore_IntentionDelete(t *testing.T) { Name: "web", Sources: []*structs.SourceIntention{ { - LegacyID: id, - Name: "*", - Action: structs.IntentionActionAllow, + LegacyID: id, + Name: "*", + Action: structs.IntentionActionAllow, + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, }, }, } @@ -519,6 +1091,8 @@ func TestStore_IntentionsList(t *testing.T) { SourceType: structs.IntentionSourceConsul, Action: structs.IntentionActionAllow, Meta: map[string]string{}, + CreatedAt: testTimeA, + UpdatedAt: testTimeA, } } @@ -530,22 +1104,17 @@ func TestStore_IntentionsList(t *testing.T) { id := testUUID() for _, src := range srcs { conf.Sources = append(conf.Sources, &structs.SourceIntention{ - LegacyID: id, - Name: src, - Action: structs.IntentionActionAllow, + LegacyID: id, + Name: src, + Action: structs.IntentionActionAllow, + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, }) } return conf } - cmpIntention := func(ixn *structs.Intention, id string) *structs.Intention { - ixn.ID = id - //nolint:staticcheck - ixn.UpdatePrecedence() - return ixn - } - - clearIrrelevantFields := func(ixns []*structs.Intention) { + clearIrrelevantFields := func(ixns ...*structs.Intention) { // Clear fields irrelevant for comparison. for _, ixn := range ixns { ixn.Hash = nil @@ -556,6 +1125,15 @@ func TestStore_IntentionsList(t *testing.T) { } } + cmpIntention := func(ixn *structs.Intention, id string) *structs.Intention { + ixn2 := ixn.Clone() + ixn2.ID = id + clearIrrelevantFields(ixn2) + //nolint:staticcheck + ixn2.UpdatePrecedence() + return ixn2 + } + var ( expectIDs []string ) @@ -610,7 +1188,7 @@ func TestStore_IntentionsList(t *testing.T) { require.Equal(t, !legacy, fromConfig) require.Equal(t, lastIndex, idx) - clearIrrelevantFields(actual) + clearIrrelevantFields(actual...) require.Equal(t, expected, actual) }) } diff --git a/agent/structs/config_entry_intentions.go b/agent/structs/config_entry_intentions.go index f863c378f0..7a737d67d3 100644 --- a/agent/structs/config_entry_intentions.go +++ b/agent/structs/config_entry_intentions.go @@ -405,6 +405,9 @@ func (e *ServiceIntentionsConfigEntry) normalize(legacyWrite bool) error { return fmt.Errorf("config entry is nil") } + // NOTE: this function must be deterministic so that the raft log doesn't + // diverge. This means no ID assignments or time.Now() usage! + e.Kind = ServiceIntentions e.EnterpriseMeta.Normalize() @@ -430,11 +433,6 @@ func (e *ServiceIntentionsConfigEntry) normalize(legacyWrite bool) error { if src.LegacyMeta == nil { src.LegacyMeta = make(map[string]string) } - // Set the created/updated times. If this is an update instead of an insert - // the UpdateOver() will fix it up appropriately. - now := time.Now().UTC() - src.LegacyCreateTime = timePointer(now) - src.LegacyUpdateTime = timePointer(now) } else { // Legacy fields are cleared, except LegacyMeta which we leave // populated so that we can later fail the write in Validate() and @@ -594,11 +592,25 @@ func (e *ServiceIntentionsConfigEntry) validate(legacyWrite bool) error { ) } } + + if src.LegacyCreateTime == nil { + return fmt.Errorf("Sources[%d].LegacyCreateTime must be set", i) + } + if src.LegacyUpdateTime == nil { + return fmt.Errorf("Sources[%d].LegacyUpdateTime must be set", i) + } } else { if len(src.LegacyMeta) > 0 { return fmt.Errorf("Sources[%d].LegacyMeta must be omitted", i) } src.LegacyMeta = nil // ensure it's completely unset + + if src.LegacyCreateTime != nil { + return fmt.Errorf("Sources[%d].LegacyCreateTime must be omitted", i) + } + if src.LegacyUpdateTime != nil { + return fmt.Errorf("Sources[%d].LegacyUpdateTime must be omitted", i) + } } if legacyWrite { diff --git a/agent/structs/config_entry_intentions_test.go b/agent/structs/config_entry_intentions_test.go index 1d84c12a8c..7660f77796 100644 --- a/agent/structs/config_entry_intentions_test.go +++ b/agent/structs/config_entry_intentions_test.go @@ -21,6 +21,14 @@ func generateUUID() (ret string) { } func TestServiceIntentionsConfigEntry(t *testing.T) { + var ( + testLocation = time.FixedZone("UTC-8", -8*60*60) + + testTimeA = time.Date(1955, 11, 5, 6, 15, 0, 0, testLocation) + testTimeB = time.Date(1985, 10, 26, 1, 35, 0, 0, testLocation) + testTimeC = time.Date(2015, 10, 21, 16, 29, 0, 0, testLocation) + ) + type testcase struct { entry *ServiceIntentionsConfigEntry legacy bool @@ -126,9 +134,11 @@ func TestServiceIntentionsConfigEntry(t *testing.T) { Name: "test", Sources: []*SourceIntention{ { - LegacyID: legacyIDs[0], - Name: "foo", - Action: IntentionActionAllow, + LegacyID: legacyIDs[0], + Name: "foo", + Action: IntentionActionAllow, + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, }, }, Meta: map[string]string{ @@ -198,10 +208,12 @@ func TestServiceIntentionsConfigEntry(t *testing.T) { Name: "test", Sources: []*SourceIntention{ { - LegacyID: legacyIDs[0], - Name: "foo", - Action: IntentionActionAllow, - Description: strings.Repeat("x", 512), + LegacyID: legacyIDs[0], + Name: "foo", + Action: IntentionActionAllow, + Description: strings.Repeat("x", 512), + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, LegacyMeta: map[string]string{ // stray Meta will be dropped "old": "data", }, @@ -217,10 +229,12 @@ func TestServiceIntentionsConfigEntry(t *testing.T) { Name: "test", Sources: []*SourceIntention{ { - LegacyID: legacyIDs[0], - Name: "foo", - Action: IntentionActionAllow, - LegacyMeta: makeStringMap(65, 5, 5), + LegacyID: legacyIDs[0], + Name: "foo", + Action: IntentionActionAllow, + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, + LegacyMeta: makeStringMap(65, 5, 5), }, }, }, @@ -233,10 +247,12 @@ func TestServiceIntentionsConfigEntry(t *testing.T) { Name: "test", Sources: []*SourceIntention{ { - LegacyID: legacyIDs[0], - Name: "foo", - Action: IntentionActionAllow, - LegacyMeta: makeStringMap(64, 129, 5), + LegacyID: legacyIDs[0], + Name: "foo", + Action: IntentionActionAllow, + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, + LegacyMeta: makeStringMap(64, 129, 5), }, }, }, @@ -249,10 +265,12 @@ func TestServiceIntentionsConfigEntry(t *testing.T) { Name: "test", Sources: []*SourceIntention{ { - LegacyID: legacyIDs[0], - Name: "foo", - Action: IntentionActionAllow, - LegacyMeta: makeStringMap(64, 128, 513), + LegacyID: legacyIDs[0], + Name: "foo", + Action: IntentionActionAllow, + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, + LegacyMeta: makeStringMap(64, 128, 513), }, }, }, @@ -265,10 +283,12 @@ func TestServiceIntentionsConfigEntry(t *testing.T) { Name: "test", Sources: []*SourceIntention{ { - LegacyID: legacyIDs[0], - Name: "foo", - Action: IntentionActionAllow, - LegacyMeta: makeStringMap(64, 128, 512), + LegacyID: legacyIDs[0], + Name: "foo", + Action: IntentionActionAllow, + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, + LegacyMeta: makeStringMap(64, 128, 512), }, }, }, @@ -280,9 +300,11 @@ func TestServiceIntentionsConfigEntry(t *testing.T) { Name: "test", Sources: []*SourceIntention{ { - Name: "foo", - Action: IntentionActionAllow, - Description: strings.Repeat("x", 512), + Name: "foo", + Action: IntentionActionAllow, + Description: strings.Repeat("x", 512), + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, }, }, }, @@ -1008,8 +1030,10 @@ func TestServiceIntentionsConfigEntry(t *testing.T) { Action: IntentionActionDeny, }, { - Name: "foo", - Action: IntentionActionAllow, + Name: "foo", + Action: IntentionActionAllow, + LegacyCreateTime: &testTimeA, // stray times will be dropped + LegacyUpdateTime: &testTimeA, }, { Name: "bar", @@ -1059,9 +1083,11 @@ func TestServiceIntentionsConfigEntry(t *testing.T) { Name: "test", Sources: []*SourceIntention{ { - Name: WildcardSpecifier, - Action: IntentionActionDeny, - LegacyID: legacyIDs[0], + Name: WildcardSpecifier, + Action: IntentionActionDeny, + LegacyID: legacyIDs[0], + LegacyCreateTime: &testTimeA, + LegacyUpdateTime: &testTimeA, }, { Name: "foo", @@ -1071,23 +1097,27 @@ func TestServiceIntentionsConfigEntry(t *testing.T) { "key1": "val1", "key2": "val2", }, + LegacyCreateTime: &testTimeB, + LegacyUpdateTime: &testTimeB, }, { - Name: "bar", - Action: IntentionActionDeny, - LegacyID: legacyIDs[2], + Name: "bar", + Action: IntentionActionDeny, + LegacyID: legacyIDs[2], + LegacyCreateTime: &testTimeC, + LegacyUpdateTime: &testTimeC, }, }, }, check: func(t *testing.T, entry *ServiceIntentionsConfigEntry) { require.Len(t, entry.Sources, 3) - assert.False(t, entry.Sources[0].LegacyCreateTime.IsZero()) - assert.False(t, entry.Sources[0].LegacyUpdateTime.IsZero()) - assert.False(t, entry.Sources[1].LegacyCreateTime.IsZero()) - assert.False(t, entry.Sources[1].LegacyUpdateTime.IsZero()) - assert.False(t, entry.Sources[2].LegacyCreateTime.IsZero()) - assert.False(t, entry.Sources[2].LegacyUpdateTime.IsZero()) + // assert.False(t, entry.Sources[0].LegacyCreateTime.IsZero()) + // assert.False(t, entry.Sources[0].LegacyUpdateTime.IsZero()) + // assert.False(t, entry.Sources[1].LegacyCreateTime.IsZero()) + // assert.False(t, entry.Sources[1].LegacyUpdateTime.IsZero()) + // assert.False(t, entry.Sources[2].LegacyCreateTime.IsZero()) + // assert.False(t, entry.Sources[2].LegacyUpdateTime.IsZero()) assert.Equal(t, []*SourceIntention{ { @@ -1299,6 +1329,8 @@ func TestMigrateIntentions(t *testing.T) { LegacyMeta: map[string]string{ "key1": "val1", }, + LegacyCreateTime: &anyTime, + LegacyUpdateTime: &anyTime, }, }, }, @@ -1349,6 +1381,8 @@ func TestMigrateIntentions(t *testing.T) { LegacyMeta: map[string]string{ "key1": "val1", }, + LegacyCreateTime: &anyTime, + LegacyUpdateTime: &anyTime, }, { LegacyID: legacyIDs[1], @@ -1359,6 +1393,8 @@ func TestMigrateIntentions(t *testing.T) { LegacyMeta: map[string]string{ "key2": "val2", }, + LegacyCreateTime: &anyTime, + LegacyUpdateTime: &anyTime, }, }, }, @@ -1409,6 +1445,8 @@ func TestMigrateIntentions(t *testing.T) { LegacyMeta: map[string]string{ "key1": "val1", }, + LegacyCreateTime: &anyTime, + LegacyUpdateTime: &anyTime, }, }, }, @@ -1425,6 +1463,8 @@ func TestMigrateIntentions(t *testing.T) { LegacyMeta: map[string]string{ "key2": "val2", }, + LegacyCreateTime: &anyTime, + LegacyUpdateTime: &anyTime, }, }, }, diff --git a/agent/structs/intention.go b/agent/structs/intention.go index 29d2811a20..2b33ddad19 100644 --- a/agent/structs/intention.go +++ b/agent/structs/intention.go @@ -440,6 +440,9 @@ func (x *Intention) ToConfigEntry(legacy bool) *ServiceIntentionsConfigEntry { } func (x *Intention) ToSourceIntention(legacy bool) *SourceIntention { + ct := x.CreatedAt // copy + ut := x.UpdatedAt + src := &SourceIntention{ Name: x.SourceName, EnterpriseMeta: *x.SourceEnterpriseMeta(), @@ -450,8 +453,8 @@ func (x *Intention) ToSourceIntention(legacy bool) *SourceIntention { Type: x.SourceType, Description: x.Description, LegacyMeta: x.Meta, - LegacyCreateTime: nil, // Ignore - LegacyUpdateTime: nil, // Ignore + LegacyCreateTime: &ct, + LegacyUpdateTime: &ut, } if !legacy { src.Permissions = x.Permissions @@ -522,13 +525,30 @@ type IntentionRequest struct { Op IntentionOp // Intention is the intention. + // + // This is mutually exclusive with the Mutation field. Intention *Intention + // Mutation is a change to make to an Intention. + // + // This is mutually exclusive with the Intention field. + // + // This field is only set by the leader before writing to the raft log and + // is not settable via the API or an RPC. + Mutation *IntentionMutation + // WriteRequest is a common struct containing ACL tokens and other // write-related common elements for requests. WriteRequest } +type IntentionMutation struct { + ID string + Destination ServiceName + Source ServiceName + Value *SourceIntention +} + // RequestDatacenter returns the datacenter for a given request. func (q *IntentionRequest) RequestDatacenter() string { return q.Datacenter