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.

Sign up for GitHub

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

Jump to bottom

Reconsider removing function signatures from auth interfaces #4535

Closed
jpkrohling opened this issue Dec 10, 2021 · 3 comments
Closed

Reconsider removing function signatures from auth interfaces #4535

jpkrohling opened this issue Dec 10, 2021 · 3 comments
Assignees
@jpkrohling

Comments

@jpkrohling
Copy link
Member

This is ready again. @bogdandrutu, there's still one point pending, about storing the HTTPInterceptorFunc signature in a type to be used by things like structs within authenticators and tests. If you agree, I would merge this and open an issue to discuss this pattern.

Originally posted by @jpkrohling in #4506 (comment)
@jpkrohling jpkrohling mentioned this issue Dec 10, 2021
Merged
jpkrohling added a commit to jpkrohling/opentelemetry-collector that referenced this issue Dec 16, 2021
@jpkrohling
Revert "Use action/checkout fetch-depth instead of manually fetch (op…
53e7e98 
…en-telemetry#4535)"

This reverts commit 5405247.
jpkrohling added a commit to jpkrohling/opentelemetry-collector that referenced this issue Dec 16, 2021
@jpkrohling
Revert "Use action/checkout fetch-depth instead of manually fetch (op…
cfedf11 
…en-telemetry#4535)"

Fixes open-telemetry#4559

This reverts commit 5405247.

Signed-off-by: Juraci Paixão Kröhling <[email protected]>
bogdandrutu pushed a commit that referenced this issue Dec 16, 2021
@jpkrohling
Revert "Use action/checkout fetch-depth instead of manually fetch (#4535
cf5aa32 
)" (#4560)

Fixes #4559

This reverts commit 5405247.

Signed-off-by: Juraci Paixão Kröhling <[email protected]>
@bogdandrutu
Copy link
Member

@jpkrohling can this be closed?

@jpkrohling
Copy link
Member Author

We still have this in the server interceptors, but I think this one is fine:

opentelemetry-collector/config/configauth/serverauth.go

Lines 42 to 44 in 51fcb65

// AuthenticateFunc defines the signature for the function responsible for performing the authentication based on the given headers map.
// See ServerAuthenticator.Authenticate.
type AuthenticateFunc func(ctx context.Context, headers map[string][]string) (context.Context, error)

If you are OK in keeping, this can be closed, as the original comment was about the HTTP one, which was indeed removed already.

@bogdandrutu
Copy link
Member

The AuthFunc is fine, my whole comments were about having the other helpers/customizations. Anyway all good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jpkrohling jpkrohling

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jpkrohling @bogdandrutu