Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: provide additional policy scopes #1569

Closed

Conversation

ronjaquensel
Copy link
Contributor

@ronjaquensel ronjaquensel commented Jun 30, 2022

What this PR changes/adds

It introduces two new policy scopes. The first one is contract.cataloging, which is used during the cataloging phase. The previously existing scope contract.negotiation is now used during the negotiation itself and to re-evaluate the policies before a new transfer process is created. The other new scope is provision.manifest.verify, which is used to evaluate a generated ResourceManifest to ensure that it fulfils the given policy.

For the purpose of evaluating ResourceManifests, also two new function interfaces are introduced: ResourceDefinitionRuleFunction and ResourceDefinitionAtomicConstraintFunction. These are analogous to the existing function interfaces, but are bound to a specific sub-type of ResourceDefinition and can return a modified version of that definition.

Why it does that

Currently, policy evaluation only takes place during the negotiation phase.

Further notes

The interfaces for the PolicyEngine and the evaluation functions are moved to the policy-spi and the function interfaces from the policy-evaluator are moved to policy-engine, to avoid circular dependencies between policy-evaluator, core-spi and transfer-spi.

Linked Issue(s)

Closes #1446
Closes #1331

Checklist

  • added appropriate tests?
  • performed checkstyle check locally?
  • added/updated copyright headers?
  • documented public classes/methods?
  • added/updated relevant documentation?
  • added relevant details to the changelog? (skip with label no-changelog)
  • formatted title correctly? (take a look at the CONTRIBUTING and styleguide for details)

Copy link
Contributor

@juliapampus juliapampus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You wrote

The first one is contract.offer-generation, which is used during the cataloging phase.

Why not call it contract.cataloging then? I am not sure whether there could be also policy enforcement during the contract.offering phase, that may need to be separated in the future. Or call it contract.offer.generation if we may have subtypes of contract.offer.**? I know this is hard to decide...

On top of that, I would detail provisioning. Cause it's resource provisioning, we could name it resource.provisioning? If we need to separate "resource" actions. Or provisioning.resource.. don't know if this is too much at the moment but we should not block expandability.

@ronjaquensel
Copy link
Contributor Author

@juliapampus you're right, they should be renamed.

Why not call it contract.cataloging then? I am not sure whether there could be also policy enforcement during the contract.offering phase, that may need to be separated in the future. Or call it contract.offer.generation if we may have subtypes of contract.offer.**? I know this is hard to decide...

Probably contract.cataloging would be the better choice, as the evaluation is technically done in the ContractDefinitionService, not the ContractOfferService. And cataloging would also imply that only the access policies, but not the contract policies are checked.

On top of that, I would detail provisioning. Cause it's resource provisioning, we could name it resource.provisioning? If we need to separate "resource" actions. Or provisioning.resource.. don't know if this is too much at the moment but we should not block expandability.

As the evaluation is done after the ResourceManifest has been created but before the actual provisioning happens, how about provision.manifest.verify or something similar?

@codecov-commenter
Copy link

codecov-commenter commented Jul 1, 2022

Codecov Report

Merging #1569 (d47c09a) into main (f325721) will decrease coverage by 0.02%.
The diff coverage is 75.28%.

@@            Coverage Diff             @@
##             main    #1569      +/-   ##
==========================================
- Coverage   67.93%   67.91%   -0.03%     
==========================================
  Files         783      783              
  Lines       16828    16910      +82     
  Branches     1068     1091      +23     
==========================================
+ Hits        11432    11484      +52     
- Misses       4922     4946      +24     
- Partials      474      480       +6     
Impacted Files Coverage Δ
...connector/transfer/core/CoreTransferExtension.java 0.00% <0.00%> (ø)
...econnector/core/base/policy/PolicyContextImpl.java 46.15% <25.00%> (-9.41%) ⬇️
...ract/validation/ContractValidationServiceImpl.java 54.54% <35.00%> (-7.00%) ⬇️
...ceconnector/core/base/policy/PolicyEngineImpl.java 78.26% <53.84%> (-3.10%) ⬇️
.../core/provision/ResourceManifestEvaluatorImpl.java 83.48% <83.48%> (ø)
...ceconnector/contract/ContractServiceExtension.java 95.71% <100.00%> (ø)
.../contract/offer/ContractDefinitionServiceImpl.java 100.00% <100.00%> (ø)
...aspaceconnector/policy/engine/PolicyEvaluator.java 76.19% <100.00%> (ø)
.../core/provision/ResourceManifestGeneratorImpl.java 100.00% <100.00%> (ø)
...sfer/core/transfer/TransferProcessManagerImpl.java 82.75% <100.00%> (+0.40%) ⬆️
... and 25 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 f325721...d47c09a. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Jul 9, 2022

This pull request is stale because it has been open for 7 days with no activity.

@github-actions github-actions bot added the stale Open for x days with no activity label Jul 9, 2022
@ronjaquensel ronjaquensel removed the stale Open for x days with no activity label Jul 11, 2022
@juliapampus juliapampus changed the title feature: additional policy scopes feat: provide additional policy scopes Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Policy enforcement: definition of policy scopes Contract policy evaluated during catalog request
3 participants