feat: UCAN 1.0#2
Conversation
frrist
left a comment
There was a problem hiding this comment.
getting some early comments in, looks great so far! just a lot to review 😅
| func doRegister(cmd *cobra.Command, args []string) error { | ||
| c, _, _, id := lib.InitClient(cmd) | ||
| c, _, _, _ := lib.InitClient(cmd) | ||
|
|
||
| endpoint, err := url.Parse(args[0]) | ||
| id, err := did.Parse(args[0]) | ||
| cobra.CheckErr(err) | ||
|
|
||
| proof, err := delegation.Parse(args[1]) | ||
| endpoint, err := url.Parse(args[1]) | ||
| cobra.CheckErr(err) | ||
|
|
||
| _, err = c.AdminProviderRegister(cmd.Context(), id.Signer, endpoint.String(), proof) | ||
| _, err = c.AdminProviderRegister(cmd.Context(), id, endpoint.String()) | ||
| cobra.CheckErr(err) | ||
|
|
||
| cmd.Println("Provider registered successfully") | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Lets remove cobra.CheckErr calls here and just return the error to the caller. registerCmd invokes doRegister as a RunE method which expects an error type. The behavior of CheckErr isn't what we want here, I think: https://github.com/spf13/cobra/blob/v1.10.2/cobra.go#L235
More generally, lets really try and limit our usage of cobra.CheckErr to init methods, flag binding, and top level CLI packages.
| client.WithServiceURL(fmt.Sprintf("http://%s:%d", cfg.Server.Host, cfg.Server.Port)), | ||
| ) | ||
| endpoint, err := url.Parse(fmt.Sprintf("http://%s:%d", cfg.Server.Host, cfg.Server.Port)) | ||
| cobra.CheckErr(err) |
There was a problem hiding this comment.
| if c.signer.DID() != c.uploadServiceID { | ||
| return nil, fmt.Errorf("admin operation not permitted: signer DID %s does not match upload service ID %s", c.signer.DID(), c.uploadServiceID) |
There was a problem hiding this comment.
Thought: Feels like this ought to be a middleware since this is ~ACL of who/what can call these methods.
| fields := []zap.Field{ | ||
| zap.Stringer("issuer", inv.Issuer().DID()), | ||
| zap.Stringer("subject", inv.Subject().DID()), | ||
| zap.Stringer("command", inv.Command()), | ||
| zap.Any("arguments", inv.Arguments()), | ||
| } | ||
| if inv.Audience() != nil { | ||
| fields = append(fields, zap.Stringer("audience", inv.Audience().DID())) | ||
| } | ||
| if len(inv.Metadata()) > 0 { | ||
| fields = append(fields, zap.Any("metadata", inv.Metadata())) | ||
| } | ||
| if len(inv.Proofs()) > 0 { | ||
| fields = append(fields, zap.Stringers("proofs", inv.Proofs())) | ||
| } | ||
| log := logger.With(zap.Dict("invocation", fields...)) |
There was a problem hiding this comment.
https://github.com/uber-go/zap/blob/master/zapcore/marshaler.go#L23-L32 is nice for this type of thing, lets us re-use it across calls.
Upgrades ucantone and makes changes here to support the deferred argument/metadata types. refs fil-forge/ucantone#8
Adds a "proof store" interface and implementation allowing you to create one from a UCAN container. The interface is a thin wrapper around the lower level utility functions for obtaining proof chains (and attestations) for invocations. Note: this was implemented in fil-forge/sprue#2 originally but I need it also in indexing service. Renames `capabilities` to `commands` and renames `capabilities.MustNew(...)` to `capabilities.MustParse(...)`. depends on fil-forge/ucantone#14
Moves the code base to UCAN 1.0, as implemented by https://github.com/fil-forge/ucantone.
Note: Does not implement replication yet.