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

refactor: Rework OPA policies with go 'embed' #1018

Merged

Conversation

denis-tingaikin
Copy link
Member

@denis-tingaikin denis-tingaikin commented Jul 13, 2021

Signed-off-by: denis-tingajkin denis.tingajkin@xored.com

Description

Motivation

  1. Closes Use go:embed from go 1.16 to store OPA policies in separate files #781
  2. Removes token responsibility from updatepath. Now updatepath is just updating path.

How Has This Been Tested?

  • Added unit testing to cover
  • Tested manually
  • Tested by integration testing
  • Have not tested

Types of changes

  • Bug fix
  • New functionallity
  • Documentation
  • Refactoring
  • CI

@denis-tingaikin denis-tingaikin marked this pull request as draft July 14, 2021 16:13
@denis-tingaikin denis-tingaikin marked this pull request as ready for review July 17, 2021 14:00
@@ -103,16 +103,16 @@ func NewServer(ctx context.Context, tokenGenerator token.GeneratorFunc, options
rv.NetworkServiceServer = chain.NewNamedNetworkServiceServer(
opts.name,
append([]networkservice.NetworkServiceServer{
serialize.NewServer(),

Choose a reason for hiding this comment

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

serialize cannot stand before updatepath - in such case all Request/Close events generated from refresh/timeout/heal will race with Request/Close coming from previous hop, because they will have different Connection.Id.

Copy link
Member Author

@denis-tingaikin denis-tingaikin Aug 8, 2021

Choose a reason for hiding this comment

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

It seems as a problem with serialize . But ok, I'll fix sequence because @edwarnicke planning to replace serialize to begin.

Copy link
Member Author

@denis-tingaikin denis-tingaikin Aug 8, 2021

Choose a reason for hiding this comment

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

@Bolodya1997 Do you have an idea why all tests working fine when serialize is first in the chain? Did we miss scenario coverage tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

We don't have sandbox tests for timeout/refresh overlapping with Close/Request coming from the Client - it is pretty hard to guarantee such case without using clock in sandbox.

@denis-tingaikin denis-tingaikin force-pushed the refactor-opa-to-use-embed branch 2 times, most recently from 3d9c610 to 86e542c Compare August 8, 2021 20:27
Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>
Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>
Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>
Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>
Signed-off-by: denis-tingaikin <denis.tingajkin@xored.com>
@edwarnicke edwarnicke merged commit 9c4ad0c into networkservicemesh:main Aug 19, 2021
nsmbot pushed a commit to networkservicemesh/cmd-nse-vfio that referenced this pull request Aug 19, 2021
…k@main

PR link: networkservicemesh/sdk#1018

Commit: 9c4ad0c
Author: Denis Tingaikin
Date: 2021-08-19 16:52:56 +0300
Message:
  - refactor: Rework OPA policies with go 'embed' (#1018)
* rework OPA policies with go embed

Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

* move tokens adding from updatepath to upatetoken

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* fix linter issue

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nsc-init that referenced this pull request Aug 19, 2021
…k@main

PR link: networkservicemesh/sdk#1018

Commit: 9c4ad0c
Author: Denis Tingaikin
Date: 2021-08-19 16:52:56 +0300
Message:
  - refactor: Rework OPA policies with go 'embed' (#1018)
* rework OPA policies with go embed

Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

* move tokens adding from updatepath to upatetoken

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* fix linter issue

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-registry-memory that referenced this pull request Aug 19, 2021
…k@main

PR link: networkservicemesh/sdk#1018

Commit: 9c4ad0c
Author: Denis Tingaikin
Date: 2021-08-19 16:52:56 +0300
Message:
  - refactor: Rework OPA policies with go 'embed' (#1018)
* rework OPA policies with go embed

Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

* move tokens adding from updatepath to upatetoken

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* fix linter issue

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nse-icmp-responder that referenced this pull request Aug 19, 2021
…k@main

PR link: networkservicemesh/sdk#1018

Commit: 9c4ad0c
Author: Denis Tingaikin
Date: 2021-08-19 16:52:56 +0300
Message:
  - refactor: Rework OPA policies with go 'embed' (#1018)
* rework OPA policies with go embed

Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

* move tokens adding from updatepath to upatetoken

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* fix linter issue

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nsmgr that referenced this pull request Aug 19, 2021
…k@main

PR link: networkservicemesh/sdk#1018

Commit: 9c4ad0c
Author: Denis Tingaikin
Date: 2021-08-19 16:52:56 +0300
Message:
  - refactor: Rework OPA policies with go 'embed' (#1018)
* rework OPA policies with go embed

Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

* move tokens adding from updatepath to upatetoken

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* fix linter issue

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nsmgr-proxy that referenced this pull request Aug 19, 2021
…k@main

PR link: networkservicemesh/sdk#1018

Commit: 9c4ad0c
Author: Denis Tingaikin
Date: 2021-08-19 16:52:56 +0300
Message:
  - refactor: Rework OPA policies with go 'embed' (#1018)
* rework OPA policies with go embed

Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

* move tokens adding from updatepath to upatetoken

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* fix linter issue

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/sdk-k8s that referenced this pull request Aug 19, 2021
…k@main

PR link: networkservicemesh/sdk#1018

Commit: 9c4ad0c
Author: Denis Tingaikin
Date: 2021-08-19 16:52:56 +0300
Message:
  - refactor: Rework OPA policies with go 'embed' (#1018)
* rework OPA policies with go embed

Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

* move tokens adding from updatepath to upatetoken

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* fix linter issue

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/sdk-kernel that referenced this pull request Aug 19, 2021
…k@main

PR link: networkservicemesh/sdk#1018

Commit: 9c4ad0c
Author: Denis Tingaikin
Date: 2021-08-19 16:52:56 +0300
Message:
  - refactor: Rework OPA policies with go 'embed' (#1018)
* rework OPA policies with go embed

Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

* move tokens adding from updatepath to upatetoken

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* fix linter issue

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-registry-proxy-dns that referenced this pull request Aug 19, 2021
…k@main

PR link: networkservicemesh/sdk#1018

Commit: 9c4ad0c
Author: Denis Tingaikin
Date: 2021-08-19 16:52:56 +0300
Message:
  - refactor: Rework OPA policies with go 'embed' (#1018)
* rework OPA policies with go embed

Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

* move tokens adding from updatepath to upatetoken

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* fix linter issue

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* apply review comments

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
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.

Use go:embed from go 1.16 to store OPA policies in separate files
4 participants