Use implicit field mask paths in IS

[htdvisser]

Summary:

Since #69 (specifically the decisions in #69 (comment) and related meeting) we now allow empty field masks. This PR makes sure that the Identity Server handles such requests accordingly.

Changes:

• Field Mask paths on IS requests are "cleaned" according to a set of possible/allowed fields, as well as fields to add (default fields on Get-like requests) or remove (ignored fields on Set-like requests)
• CLI is updated to no longer implicitly select the "name" field.

Notes for Reviewers:

@rvolosatovs if you think it could be useful in other places (end device registries), we can move the cleanFieldMaskPaths utility to ttnpb.

Release Notes: (optional)

The Identity Server now treats empty field masks as if ids and created/updated times were requested.

[htdvisser]

Setting prio/high as this is required for 3.0.0

[rvolosatovs]

Not explicitly requesting changes/approving, since I don't understand what's going on

[rvolosatovs]

This will not evaluate to true if any of the child paths in contact_info, e.g. contact_info.contact_type is set(https://github.com/TheThingsNetwork/lorawan-stack/blob/master/api/contact_info.proto#L45)
Is that expected behavior?
Additionally, it seems like contact_info is not part of allowed fieldmasks of any PRC at https://github.com/TheThingsNetwork/lorawan-stack/blob/9e35fc3e7bf1ce817076ed39a916914e1b739de7/pkg/ttnpb/field_mask_validation.go at all.
How is this supposed to work?

[htdvisser]

1. contact_info is a slice. Field mask paths don't recurse into slices.
2. EndDevice doesn't have a contact_info field. For applications and other entities, the allowed paths come from ttnpb.ApplicationFieldPathsNested etc.

[rvolosatovs]

Why is this necessary?
Why not just set this to getPaths if len(req.FieldMask.Paths) == 0?
Basically the same question for all other invocations - I really don't get what's going on here.

[htdvisser]

getPaths contains the fields that are implicitly part of the field mask, so they are always added to the field mask in the request, if it doesn't already contain them.

[rvolosatovs]

Yeah, but there's no harm in adding a field twice - you can just append the fields - we do the same in JS/NS and AS AFAIK.

[rvolosatovs]

In order to construct the slice of required fields, Set registry endpoint in NS constructs a gets and sets:

lorawan-stack/pkg/networkserver/grpc_deviceregistry.go

Lines 49 to 57 in 59154c3

	gets := append(req.FieldMask.Paths[:0:0], req.FieldMask.Paths...)
	if ttnpb.HasAnyField(req.FieldMask.Paths, "mac_state.device_class") {
	gets = append(gets,
	"mac_state.current_parameters",
	"mac_state.desired_parameters",
	"queued_application_downlinks",
	)
	}

[htdvisser]

This is not the NS/JS. In the IS we prefer to have paths in the field mask only once.

[rvolosatovs]

But then why not ttnpb.EnsureFieldpaths(paths []string, ensure ...string) []string?
You pass the existing fieldmask and the paths you want to add if missing.
You don't need 4 arguments for that.

[htdvisser]

There is no ttnpb.EnsureFieldPaths, I want to combine this with the allowed paths, I want to be able to specify paths that need to be deleted if present, and finally, I want to do it in a single pass instead of combining EnsureFieldPaths, EnsureNotFieldPaths, IntersectFieldPaths, UniqueFieldPaths.

[rvolosatovs]

1. We have API layer middleware, which should validate that fields that should not be present are not present.
2. Invalid fieldpaths, which still could get through should be validated explicitly and if present - throw an error.

RPC implementation simply should not delete anything passed to it. Cleaning and validation belongs to the API layer middleware. RPC should simply upsert the required fields.