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

[sdk#740] Rework sandbox to use tokenTimeout instead of tokenGenerator #996

Conversation

Bolodya1997
Copy link

Description

  1. Reworks sandbox to set tokenGeneratorSupplier for domain:
// SupplyTokenGeneratorFunc supplies token generator
type SupplyTokenGeneratorFunc func(tokenTimeout time.Duration) token.GeneratorFunc
  1. Starts using tokenTimeout instead of tokenGenerator in all New* methods:
// NewNSMgr creates a new NSMgr
func (n *Node) NewNSMgr(
	ctx context.Context,
	name string,
	serveURL *url.URL,
-	tokenGenerator token.GeneratorFunc,
+	tokenTimeout time.Duration,
	supplyNSMgr SupplyNSMgrFunc,
) *NSMgrEntry {
  1. Adds DefaultDialOptions for domain:
// DefaultDialOptions returns default dial options for the domain
func (d *Domain) DefaultDialOptions(tokenTimeout time.Duration) []grpc.DialOption {
	return DefaultDialOptions(d.supplyTokenGenerator(tokenTimeout))
}

Issue link

How Has This Been Tested?

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

Types of changes

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

Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Copy link
Member

@denis-tingaikin denis-tingaikin left a comment

Choose a reason for hiding this comment

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

I don't think that we need to replace tokenGen function with token timeout in sandbox due to it can be used for advanced OPA policy testing.

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.

2 participants