Introduce a new service: ape_manager
#1105
Labels
No labels
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No milestone
No project
No assignees
4 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-node#1105
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "aarifullin/frostfs-node:feat/ape_manager"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The introduced service
ape_manager
manages APE policies for native-specific targets like containers.Unlike
control
service, that manages only local overrides,ape_manager
allows a client to set rule chains in Policy contract for resources it owns.This PR:
util
package;ProxyVerificationContractStorage
uses Proxy contract as a cosigner.ProxyVerificationContractStorage
has an implementation feature: it recreates a contract storage for each handler invocation because of an issue: rpc-actor from morph client may be expired. Such way won't create a bottlenecks because it is expected that this contract storage implementation will be used not so often. This approach requires to make morph client returnRPCActor
(wsClient
in fact).To implement
ape_manager
service this PR:Also, it
ape_manager
infrostfs-cli
@ -0,0 +23,4 @@
)
// ContainerAPEChainManager interface provides methods to manage rule chains for containers in
// Policy contract.
This comment will be fixed
84a56abbcb
toe2d8ca4616
Why are
*.proto
files not in the frostfs-api repo?Should they?
.proto
-s are placed beside implementationfrostfs-api
, we need to drag it over this cyclefrostfs-api -> frostfs-api-go -> frostfs-sdk-go
creating wrappers etc. But I doubt this lightweight service requires this cycle: it barely needs SDK as it won't interract with other nodesIf you convince me that tree service is a specific case, then I'll move it to
frostfs-api
e2d8ca4616
to63fa659132
Tree service is not a part of public API (yet) for a reason (unsolved problems related to security and scalability in the untrusted environment).
This service should replace eACL and thus should be a part of the public API.
Introduce a new service:to WIP: Introduce a new service:ape_manager
ape_manager
63fa659132
toe9e7f8cd95
e9e7f8cd95
to4b32432a9f
WIP: Introduce a new service:to Introduce a new service:ape_manager
ape_manager
4b32432a9f
tobac0efa68a
bac0efa68a
tobd92279e75
@ -0,0 +39,4 @@
)
var (
errUnknownTargetType = errors.New("unknown target type")
It is used only once and in the place where
%w
is essentially useless, to we need a variable?@ -0,0 +16,4 @@
errEmptyChainID = errors.New("chain id cannot be empty")
removeChainCmd = &cobra.Command{
Use: "rm-rule-chain",
We have
rm-rule-chain
,add-rule-chain
andlist-rule-chains
I don't think we should have anything besides
rule-chain
here, so why notremove
,add
andlist
?If we want to mirror api, then
rule
should be removed.In any case, I think
remove
is a better choice thanrm
(and it is in the api proto definitions too)Agreed. Using
add/list/remove
names are unambiguous asapemanager
manages only chains. So, I have renamed them@ -0,0 +21,4 @@
}
func (s *APEService) AddChain(ctx context.Context, req *apemanager_v2.AddChainRequest) (*apemanager_v2.AddChainResponse, error) {
// TODO (aarifullin): make this handler perform APE checks as soon as relevant verbs will be introduced in policy-engine.
If we have a task let's write a link here.
If not, let's remove this TODO.
TODO
s are removed@ -0,0 +22,4 @@
// APEChainManager interface provides methods to manage rule chains for containers in
// Policy contract.
type APEChainManager interface {
APEManager service is resource-agnostic, why have you decided to create a separate package for 1 out of 4 resource types?
It seems as simple as a single validation check which can be done via
switch
in the main service.I have simplified the service implementation - I have removed
container
package and moved this logic to the main service@ -0,0 +167,4 @@
}
func getSignaturePublicKey(vh *session.RequestVerificationHeader) (*keys.PublicKey, error) {
for vh.GetOrigin() != nil {
Why do we need this loop?
Oh, my bad!
I didn't notice that
GetOrigin
is being performed in the loop. But, I copied that from here:https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/container/ape.go#L421
This loop can be found in these places:
https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/v2/util.go#L63
https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/eacl/v2/xheader.go#L33
Probably, that makes sense?
@ -0,0 +7,4 @@
)
// Bytes generate random bytes.
func Bytes(n int) ([]byte, error) {
It is unused, why it is public?
Have removed
@ -0,0 +17,4 @@
}
// Base58Str generate random base58 string.
func Base58Str(n int) (string, error) {
It is used exactly once, do we need a separate package for it?
Alright. I supposed that could be useful, but it is really used in one place. So, I have removed it
bd92279e75
to6a7d5a4de9
6a7d5a4de9
to6de48cdf65
6de48cdf65
to647697a20e
647697a20e
to9fdad39811
9fdad39811
toe6d3b9b65d
@ -0,0 +8,4 @@
// APE Manager manages chains for native protocol targets like containers, but,
// as a service, it is also required to be checked with APE.
type APEService struct {
Now this wrapper seems completely useless, what about removing it?
We will properly introduce it when the time comes.
Alright. Removed
e6d3b9b65d
to61cc4b48d4
61cc4b48d4
to54bc1ee58a