asserts: define registry-control assertion #14508
Conversation
|
Everyone contributing to this PR have now signed the CLA. Thanks! |
st3v3nmw
changed the title
st3v3nmw
force-pushed
the
add-registry-control-assertion
branch
3 times, most recently
from
5149fd0
to
b47e694
Compare
st3v3nmw
force-pushed
the
add-registry-control-assertion
branch
3 times, most recently
from
234c56e
to
6a94aa1
Compare
|
Everyone contributing to this PR have now signed the CLA. Thanks! |
st3v3nmw
force-pushed
the
add-registry-control-assertion
branch
from
6a94aa1
to
318806c
Compare
registry/registry_control.go
Outdated
| } | ||
|
|
||
| // Revoke revokes remote registry control over <account-id>/<registry>/<view>. | ||
| func (rgCtrl *RegistryControl) Revoke(accountID, registryName, view string) error { |
There was a problem hiding this comment.
The return type error can be removed since the function always returns nil
registry/registry_control.go
Outdated
| // -*- Mode: Go; indent-tabs-mode: t -*- | ||
|
|
||
| /* | ||
| * Copyright (C) 2022-2024 Canonical Ltd |
There was a problem hiding this comment.
| * Copyright (C) 2022-2024 Canonical Ltd | |
| * Copyright (C) 2024 Canonical Ltd |
registry/registry_control.go
Outdated
| accountID := parts[0] | ||
| registryName := parts[1] | ||
| viewName := parts[2] |
There was a problem hiding this comment.
nitpick: these are short enough that we can condense this a bit
| accountID := parts[0] | |
| registryName := parts[1] | |
| viewName := parts[2] | |
| accountID, registryName, viewName := parts[0], parts[1], parts[2] |
registry/registry_control_test.go
Outdated
| // -*- Mode: Go; indent-tabs-mode: t -*- | ||
|
|
||
| /* | ||
| * Copyright (C) 2022-2024 Canonical Ltd |
There was a problem hiding this comment.
| * Copyright (C) 2022-2024 Canonical Ltd | |
| * Copyright (C) 2024 Canonical Ltd |
| Registries: "registries", | ||
|
|
||
| Registries: "registries", | ||
| RegistryControl: "registry-control", |
There was a problem hiding this comment.
Maybe remote-registry-control or registry-delegation would be clearer for users?
There was a problem hiding this comment.
@pedronis may have an opinion
registry/registry_control_test.go
Outdated
| c.Assert(err.Error(), Equals, tc.err, cmt) | ||
| } else { | ||
| c.Assert(err, IsNil, cmt) | ||
| c.Check(rgCtrl, Not(IsNil), cmt) |
There was a problem hiding this comment.
| c.Check(rgCtrl, Not(IsNil), cmt) | |
| c.Check(rgCtrl, NotNil, cmt) |
registry/registry_control_test.go
Outdated
| c.Assert(err, NotNil) | ||
| c.Assert(err.Error(), Equals, tc.err, cmt) |
There was a problem hiding this comment.
| c.Assert(err, NotNil) | |
| c.Assert(err.Error(), Equals, tc.err, cmt) | |
| c.Assert(err, ErrorMatches, tc.err, cmt) |
| Views map[string]*delegatedView // key is the view's name | ||
| } | ||
|
|
||
| type delegatedView struct { |
There was a problem hiding this comment.
This type seems unnecessary, do we expect to add something to it? It only carries the view's name so AFAICT we don't need it?
There was a problem hiding this comment.
I added the type to match SD172 which looks like this:
views:
- name: <account-id>/<registry>/<view>
- name: <account-id>/<registry>/<view>
name is the only field currently. It was done this way to allow space for future expansion but I can't think of possible fields to add (maybe the authentication or authority fields?). I'll bring this up for discussion again. Thanks.
| type delegatedRegistry struct { | ||
| AccountID string | ||
| Name string | ||
| Views map[string]*delegatedView // key is the view's name |
There was a problem hiding this comment.
The keys and values are the same so this can be replaced with a list of view names with no loss
| rgCtrl.Registries[key] = registry | ||
| } | ||
|
|
||
| registry.Views[view] = &delegatedView{Name: view} |
There was a problem hiding this comment.
Just to clarify, this is what I'm referring to. This can be replaced with a slice or, if we ever expect this to grow very large, a map[string]bool or map[string]struct{}
st3v3nmw
force-pushed
the
add-registry-control-assertion
branch
from
318806c
to
5684cee
Compare
This PR adds the registry-control assertion to snapd (SD172). This doesn't include signing or acknowledging of said assertion as we need a different approach since we're signing with the device key - that will be the next PR. Finally, the API changes will follow (SD186).
This also introduces a
registry-controlfeature flag.