Add Flag for Custom Authenticators in Cassandra Storage #5628
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: mehul gautam <mehulsharma4786@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5628 +/- ##
==========================================
+ Coverage 96.34% 96.36% +0.01%
==========================================
Files 327 327
Lines 16016 16026 +10
==========================================
+ Hits 15431 15443 +12
+ Misses 407 405 -2
Partials 178 178
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
yurishkuro
reviewed
Jun 14, 2024
Signed-off-by: mehul gautam <mehulsharma4786@gmail.com>
yurishkuro
reviewed
Jun 14, 2024
|
@yurishkuro where can i add cli flags ? can you specify the file location where we define flags for cassendra ? |
Signed-off-by: mehul gautam <mehulsharma4786@gmail.com>
Signed-off-by: mehul gautam <mehulsharma4786@gmail.com>
Signed-off-by: mehul gautam <mehulsharma4786@gmail.com>
|
@yurishkuro where is this |
|
please write PR title according to the contributing guidelines |
hellspawn679
changed the title
hellspawn679
changed the title
done |
yurishkuro
changed the title
yurishkuro
reviewed
Jun 17, 2024
yurishkuro
reviewed
Jun 17, 2024
yurishkuro
reviewed
Jun 17, 2024
Signed-off-by: mehul gautam <mehulsharma4786@gmail.com>
Signed-off-by: mehul gautam <mehulsharma4786@gmail.com>
Signed-off-by: mehul gautam <mehulsharma4786@gmail.com>
yurishkuro
reviewed
Jun 17, 2024
pkg/cassandra/config/config.go
Outdated
| Basic BasicAuthenticator `yaml:"basic" mapstructure:",squash"` | ||
| // TODO: add more auth types | ||
| Basic BasicAuthenticator `yaml:"basic" mapstructure:",squash"` | ||
| AllowedAuthenticators []string `yaml:"allowed_authenticators" mapstructure:"allowed_authenticators"` |
There was a problem hiding this comment.
AllowedAuthenticators should be inside BasicAuthenticator struct. And lets add a comment to it
// The comma-separated list of allowed password authenticators for Cassandra.
// If none are specified, there is a default "approved" list that is used
// https://github.com/gocql/gocql/blob/34fdeebefcbf183ed7f916f931aa0586fdaa1b40/conn.go#L27
// If a non-empty list is provided, only specified authenticators are allowed.
// See also https://github.com/jaegertracing/jaeger/issues/5627#issuecomment-2174454633
Signed-off-by: mehul gautam <mehulsharma4786@gmail.com>
yurishkuro
reviewed
Jun 18, 2024
| @@ -62,6 +62,9 @@ func (*CassandraStorageIntegration) initializeCassandraFactory(t *testing.T, fla | |||
|
|
|||
| func (s *CassandraStorageIntegration) initializeCassandra(t *testing.T) { | |||
| f := s.initializeCassandraFactory(t, []string{ | |||
| "--cassandra.basic.allowed-authenticators=", | |||
| "--cassandra.password=password", | |||
| "--cassandra.username=username", | |||
There was a problem hiding this comment.
why does this work? Are these the default values for username/password in Cassandra image
There was a problem hiding this comment.
Ah, I think it's because we have no auth configured on the server, so the extra auth client might send is just getting ignored.
yurishkuro
approved these changes
Jun 18, 2024
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger:make lint testjaeger-ui:yarn lintandyarn test