Synchronise access policies with TrustOracle #1561

Waldz · 2020-01-22T09:39:34Z

Validates policy name by fetching it's rules from TrustOracle before starting service and subscribes for improvements of rules

core/policy/policy_repository.go

cmd/di_desktop.go

tadaskay · 2020-01-22T13:23:55Z

Do we really need to poll and cache access policies in node? This could be simply an on-demand call, could it not? @Waldz

core/policy/policy_repository.go

cmd/di_desktop.go

core/policy/policy_repository.go

codecov-io · 2020-01-23T01:31:20Z

Codecov Report

Merging #1561 into master will increase coverage by 0.05%.
The diff coverage is 53.78%.

@@            Coverage Diff             @@
##           master    #1561      +/-   ##
==========================================
+ Coverage   47.37%   47.43%   +0.05%     
==========================================
  Files         285      286       +1     
  Lines       11007    11091      +84     
==========================================
+ Hits         5215     5261      +46     
- Misses       5416     5454      +38     
  Partials      376      376

Impacted Files	Coverage Δ
market/service_proposal.go	`70% <ø> (ø)`	⬆️
config/flags_network.go	`0% <ø> (ø)`	⬆️
config/flags_service_shared.go	`0% <0%> (ø)`	⬆️
communication/nats/receiver.go	`39.08% <0%> (-2.39%)`	⬇️
core/policy/identity_validator.go	`0% <0%> (ø)`
core/service/stub_service.go	`76.47% <100%> (ø)`	⬆️
tequilapi/endpoints/service.go	`92.62% <100%> (-0.62%)`	⬇️
core/service/manager.go	`65.21% <55.55%> (-1.95%)`	⬇️
core/policy/policy_repository.go	`65.16% <65.16%> (ø)`
core/discovery/discovery.go	`65.54% <0%> (-0.85%)`	⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f816406...2d91355. Read the comment docs.

cmd/di_desktop.go

core/policy/policy_repository.go

Waldz · 2020-01-23T14:32:04Z

All improvements addressed

core/policy/policy_repository.go

soffokl · 2020-01-27T03:38:40Z

core/policy/policy_repository.go

+// Policy converts given value to valid policy rule
+func (pr *PolicyRepository) Policy(policyID string) market.AccessPolicy {
+	policyURL := pr.policyURL
+	if !strings.HasSuffix(policyURL, "/") {


Why should it ends by the /?
This looks like you are trying to fit existing service.
As a user I'd like to have an option to set any standard URL I want.

It can be 3rd party service, just follows this contract:

While starting service, provide Policy Oracle via:
--access-policy.address=https://testnet-trust.mysterium.network/api/v1/access-policies/
--access-policy.address:https://testnet-trust.mysterium.network/api/v1/access-policies

Provider policy IDs via:
--access-policy.list= ipinfo,wikipedia

So yes, provided custom URL still have to match certain contract

But you don't need to add / at the end of the user URL.
If you need it for URL https://testnet-trust.mysterium.network/api/v1/access-policies/ you passing it through the command argument.

If I need to pass the URL https://myown-trust.com/policies.json, it should not become https://myown-trust.com/policies.json/.

tadaskay

Added a failing test case

core/policy/policy_repository.go

tadaskay · 2020-01-27T08:33:22Z

core/policy/policy_repository.go

+}
+
+// RulesForPolicy gives rules of given policy
+func (pr *PolicyRepository) RulesForPolicy(policy market.AccessPolicy) (market.AccessPolicyRuleSet, error) {


It appears there are no differences between RulesForPolicy and RulesForPolicies. Perhaps merge into one RulesForPolicies(policies ...market.AccessPolicy)?

Actually there is difference in return type, multiple retrieve returns list of asked rulesets

core/policy/repository.go

go test -run Test_PolicyRepository_AddPolicies_Race github.com/mysteriumnetwork/node/core/policy

Waldz requested review from chompomonim, anjmao, tadaskay, soffokl, zolia and vkuznecovas January 22, 2020 09:39

Waldz requested a review from tadovas as a code owner January 22, 2020 09:39

Waldz force-pushed the feature/sync-policy-rules branch from 63c03cf to 531199d Compare January 22, 2020 09:39

soffokl requested changes Jan 22, 2020

View reviewed changes

core/policy/policy_repository.go Outdated Show resolved Hide resolved

anjmao suggested changes Jan 22, 2020

View reviewed changes

core/policy/policy_repository.go Outdated Show resolved Hide resolved

cmd/di_desktop.go Outdated Show resolved Hide resolved

anjmao suggested changes Jan 22, 2020

View reviewed changes

core/policy/policy_repository.go Outdated Show resolved Hide resolved

cmd/di_desktop.go Outdated Show resolved Hide resolved

core/policy/policy_repository.go Outdated Show resolved Hide resolved

core/policy/policy_repository.go Outdated Show resolved Hide resolved

Waldz force-pushed the feature/sync-policy-rules branch 4 times, most recently from 47727e5 to 1ffe2ec Compare January 23, 2020 01:18

vkuznecovas suggested changes Jan 23, 2020

View reviewed changes

cmd/di_desktop.go Outdated Show resolved Hide resolved

core/policy/policy_repository.go Outdated Show resolved Hide resolved

core/policy/policy_repository.go Outdated Show resolved Hide resolved

Waldz force-pushed the feature/sync-policy-rules branch from 1ffe2ec to 7ffe4f3 Compare January 23, 2020 14:31

Waldz requested review from anjmao, soffokl and vkuznecovas January 23, 2020 14:31

vkuznecovas suggested changes Jan 24, 2020

View reviewed changes

core/policy/policy_repository.go Outdated Show resolved Hide resolved

anjmao reviewed Jan 24, 2020

View reviewed changes

core/policy/policy_repository.go Outdated Show resolved Hide resolved

anjmao reviewed Jan 24, 2020

View reviewed changes

core/policy/policy_repository.go Outdated Show resolved Hide resolved

Waldz force-pushed the feature/sync-policy-rules branch from 1f4458b to 2d91355 Compare January 24, 2020 17:05

soffokl approved these changes Jan 27, 2020

View reviewed changes

tadaskay requested changes Jan 27, 2020

View reviewed changes

Waldz requested a review from anjmao January 27, 2020 13:28

Waldz requested review from tadaskay and vkuznecovas January 27, 2020 13:29

vkuznecovas approved these changes Jan 27, 2020

View reviewed changes

Waldz force-pushed the feature/sync-policy-rules branch from ab103b7 to 765225a Compare January 27, 2020 17:56

anjmao reviewed Jan 28, 2020

View reviewed changes

core/policy/repository.go Outdated Show resolved Hide resolved

core/policy/repository.go Show resolved Hide resolved

Waldz force-pushed the feature/sync-policy-rules branch from 765225a to 95fe634 Compare January 28, 2020 11:03

Waldz and others added 14 commits January 28, 2020 14:52

Synchronise access policies with TrustOracle

283e6e9

Exception for service who don't set policies

0bc7f29

Structure of domains rules

f96881b

Improve subscription clearing logs

feece79

Improve stop channel

6b34f17

Stop policy rules syncing then node stops

472203e

Make policy fetch interval configurable

0ac3e55

Drop map locking from policy repository

281df49

Cache policy rule responses with etag

b306a10

Improve policy rules locking during update

7036622

Add test case to demonstrate race condition in addPolicies

1b8d8a3

go test -run Test_PolicyRepository_AddPolicies_Race github.com/mysteriumnetwork/node/core/policy

Handle simultaneous call to PolicyRepository.AddPolicies()

671aa4d

Rename policy.PolicyRepository -> policy.Repository

952a4eb

Solve race condition between AddPolicies() ans background sync

02b6b7f

Waldz force-pushed the feature/sync-policy-rules branch from 95fe634 to 02b6b7f Compare January 28, 2020 12:52

Merge branch 'master' into feature/sync-policy-rules

85e71e7

anjmao approved these changes Jan 29, 2020

View reviewed changes

Waldz merged commit 0c45ddb into master Jan 29, 2020

Waldz deleted the feature/sync-policy-rules branch January 29, 2020 10:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Synchronise access policies with TrustOracle #1561

Synchronise access policies with TrustOracle #1561

Waldz commented Jan 22, 2020

tadaskay commented Jan 22, 2020

codecov-io commented Jan 23, 2020 •

edited

Waldz commented Jan 23, 2020

soffokl Jan 27, 2020

Waldz Jan 27, 2020

soffokl Jan 28, 2020

tadaskay left a comment

tadaskay Jan 27, 2020

Waldz Jan 27, 2020

Synchronise access policies with TrustOracle #1561

Synchronise access policies with TrustOracle #1561

Conversation

Waldz commented Jan 22, 2020

tadaskay commented Jan 22, 2020

codecov-io commented Jan 23, 2020 • edited

Codecov Report

Waldz commented Jan 23, 2020

soffokl Jan 27, 2020

Choose a reason for hiding this comment

Waldz Jan 27, 2020

Choose a reason for hiding this comment

soffokl Jan 28, 2020

Choose a reason for hiding this comment

tadaskay left a comment

Choose a reason for hiding this comment

tadaskay Jan 27, 2020

Choose a reason for hiding this comment

Waldz Jan 27, 2020

Choose a reason for hiding this comment

codecov-io commented Jan 23, 2020 •

edited