Added db password in cassandra-integration-test #5644

ayushrakesh · 2024-06-18T07:21:46Z

Which problem is this PR solving?

Resolves Use db password in Cassandra integration tests #5643

Description of the changes

Modified the usage function to include the <db_password> argument
In check_arg function , checked the required no. of parameters - <cassandra_version>, <schema_version>, <jaeger_version>, and <db_password>, and passed to apply_schema function and handling it in main.

How was this change tested?

make test

Checklist

I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
I have signed all commits
I have added unit tests for the new functionality
I have run lint and test steps successfully
- for jaeger: make lint test
- for jaeger-ui: yarn lint and yarn test

Signed-off-by: ayushrakesh <[email protected]>

hellspawn679 · 2024-06-18T10:21:01Z

@ayushrakesh do we really need to increase the number of arguments passed this will cause the ci to fail and I don't think we should add password in github action
we can simply specify it in like we have done with primaryKeyspace

ayushrakesh · 2024-06-18T10:37:23Z

@hellspawn679 should I use env variable?

ayushrakesh · 2024-06-18T12:00:15Z

@hellspawn679 should I use env variable?

And further passing database password through Github action secrets -> in cassandra CI?

yurishkuro · 2024-06-18T16:31:21Z

do we really need to increase the number of arguments passed this will cause the ci to fail and I don't think we should add password in github action we can simply specify it in like we have done with primaryKeyspace

+1. There is no need to complicate the CI scripts with ability to override password, if it's needed in the future we can add it. It still should not be required argument to the script, but an optional one.

ayushrakesh · 2024-06-19T09:18:33Z

@yurishkuro @hellspawn679 so what should be done, db password should be passed as an optional parameter then what is the default value? Is it should be directly declared locally in run_integration function?

yurishkuro · 2024-06-22T01:18:08Z

the docker command starting Cassandra may need to specify password. The same password should be hardcoded in the integration tests. We have two flavors of integration tests:

v1 (plugin/storage/integration) which are driven from code, and the password can be passed from code
v2 (cmd/jaeger/internal/integration) is accessing storage from our main binary, and password can be passed via configuration file cmd/jaeger/config-cassandra.yaml

ayushrakesh · 2024-06-22T12:26:17Z

index: tags: true logs: true process_tags: true password: "your_password_here"

@yurishkuro like this in both cassandra main and archive in cmd/jaeger/config-cassandra.yaml ? And in plugin/storage/integration/cassandra_test.go
using env variable?

yurishkuro · 2024-06-22T14:56:02Z

As approach - yes. Exact syntax- you need to try.

Signed-off-by: ayushrakesh <[email protected]>

ayushrakesh · 2024-06-22T17:20:34Z

@yurishkuro Need to review, I have added different passwords for cassandra main and archive in cmd/jaeger/cassandra-config.yaml.

yurishkuro · 2024-06-22T20:14:35Z

scripts/cassandra-integration-test.sh

@@ -37,6 +37,7 @@ apply_schema() {
    --env CQLSH_PORT=9042
    --env "TEMPLATE=/cassandra-schema/${schema_version}.cql.tmpl"
    --env "KEYSPACE=${keyspace}"
+    --env "DB_PASSWORD=${DB_PASSWORD}"


why only password and not user name?

we don't need any env variables, the pwd can be simply hardcoded here, since the db container is created just for this test

@yurishkuro Then what is the password?

whatever, make up a string, e.g. testpwd

Signed-off-by: ayushrakesh <[email protected]>

yurishkuro · 2024-06-23T20:12:49Z

.github/workflows/ci-cassandra.yml

-      run: bash scripts/cassandra-integration-test.sh ${{ matrix.version.major }} ${{ matrix.version.schema }} ${{ matrix.jaeger-version }}
+      env:
+        DB_PASSWORD: ${{ secrets.DB_PASSWORD }}
+      run: bash scripts/cassandra-integration-test.sh ${{ matrix.version.major }} ${{ matrix.version.schema }} ${{ matrix.jaeger-version }} $DB_PASSWORD


we don't need to pass pwd here, it can be hardcoded in the script

@yurishkuro PR is revised now.

Signed-off-by: ayushrakesh <[email protected]>

codecov · 2024-06-25T22:40:37Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.94%. Comparing base (44484ac) to head (6338bf9).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5644      +/-   ##
==========================================
- Coverage   96.38%   95.94%   -0.44%     
==========================================
  Files         329      329              
  Lines       16060    16060              
==========================================
- Hits        15479    15409      -70     
- Misses        404      482      +78     
+ Partials      177      169       -8

Flag	Coverage Δ
badger_v1	`8.04% <ø> (ø)`
badger_v2	`1.92% <ø> (ø)`
cassandra-3.x-v1	`?`
cassandra-3.x-v2	`?`
cassandra-4.x-v1	`?`
cassandra-4.x-v2	`?`
elasticsearch-7.x-v1	`18.88% <ø> (+0.01%)`	⬆️
elasticsearch-8.x-v1	`19.08% <ø> (ø)`
elasticsearch-8.x-v2	`19.08% <ø> (ø)`
grpc_v1	`9.46% <ø> (-0.02%)`	⬇️
grpc_v2	`7.49% <ø> (ø)`
kafka	`9.76% <ø> (ø)`
opensearch-1.x-v1	`18.93% <ø> (+0.01%)`	⬆️
opensearch-2.x-v1	`18.92% <ø> (-0.02%)`	⬇️
opensearch-2.x-v2	`18.93% <ø> (+0.01%)`	⬆️
unittests	`94.22% <ø> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ayushrakesh · 2024-06-30T05:47:51Z

@yurishkuro Please review my revised PR.

yurishkuro · 2024-06-30T18:02:03Z

.github/workflows/ci-cassandra.yml

@@ -34,13 +34,13 @@ jobs:
      uses: step-security/harden-runner@17d0e2bd7d51742c71671bd19fa12bdc9d40a3d6 # v2.8.1
      with:
        egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs
-
+        


exclude this file from the PR, since it's not changing (git checkout main <file>)

yurishkuro

Please provide proof of successful testing with scripts/cassandra-integration-test.sh script.

yurishkuro · 2024-06-30T18:02:57Z

docker-compose/cassandra/v3/docker-compose.yaml

@@ -6,6 +6,8 @@ services:
    ports:
      - "9042:9042"
      - "9160:9160"
+    environment:
+      - DB_PASSWORD=${DB_PASSWORD}  


what about username?

yurishkuro · 2024-06-30T18:04:13Z

docker-compose/cassandra/v4/docker-compose.yaml

@@ -6,6 +6,8 @@ services:
    ports:
      - "9042:9042"
      - "9160:9160"
+    environment:
+      - DB_PASSWORD=${DB_PASSWORD}    


why DB_PASSWORD? Do you have a link to the docs showing that it's a recognized var?

yurishkuro · 2024-06-30T18:04:53Z

plugin/storage/integration/cassandra_test.go

+	require.NotEmpty(t, password, "DB_PASSWORD environment variable must be set")
+
+	// Add the Cassandra password flag
+	flags = append(flags, "--cassandra.password="+password)


this is already set in the next function, why is it here again?

yurishkuro · 2024-08-05T03:13:08Z

no follow-up, closing

Added db password in cassandra-integration-test

a7f5918

Signed-off-by: ayushrakesh <[email protected]>

ayushrakesh requested a review from a team as a code owner June 18, 2024 07:21

ayushrakesh requested a review from albertteoh June 18, 2024 07:21

Updated cassandra integration tests

ead85a5

Signed-off-by: ayushrakesh <[email protected]>

yurishkuro reviewed Jun 22, 2024

View reviewed changes

ayushrakesh and others added 2 commits June 23, 2024 14:03

Update cassandra-db integration

cb5fdd3

Signed-off-by: ayushrakesh <[email protected]>

Merge branch 'jaegertracing:main' into db-pass

a0e9811

yurishkuro reviewed Jun 23, 2024

View reviewed changes

ayushrakesh and others added 2 commits June 24, 2024 13:37

Merge branch 'jaegertracing:main' into db-pass

24adfbd

Updated test

27aeaed

Signed-off-by: ayushrakesh <[email protected]>

Merge branch 'main' into db-pass

6338bf9

yurishkuro reviewed Jun 30, 2024

View reviewed changes

Merge branch 'main' into db-pass

1c7f5c9

yurishkuro closed this Aug 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added db password in cassandra-integration-test #5644

Added db password in cassandra-integration-test #5644

ayushrakesh commented Jun 18, 2024 •

edited

Loading

hellspawn679 commented Jun 18, 2024

ayushrakesh commented Jun 18, 2024

ayushrakesh commented Jun 18, 2024

yurishkuro commented Jun 18, 2024

ayushrakesh commented Jun 19, 2024

yurishkuro commented Jun 22, 2024

ayushrakesh commented Jun 22, 2024 •

edited

Loading

yurishkuro commented Jun 22, 2024

ayushrakesh commented Jun 22, 2024

yurishkuro Jun 22, 2024

ayushrakesh Jun 23, 2024

yurishkuro Jun 23, 2024

yurishkuro Jun 23, 2024

ayushrakesh Jun 25, 2024

codecov bot commented Jun 25, 2024 •

edited

Loading

ayushrakesh commented Jun 30, 2024

yurishkuro Jun 30, 2024

yurishkuro left a comment

yurishkuro Jun 30, 2024

yurishkuro Jun 30, 2024

yurishkuro Jun 30, 2024

yurishkuro commented Aug 5, 2024

Added db password in cassandra-integration-test #5644

Added db password in cassandra-integration-test #5644

Conversation

ayushrakesh commented Jun 18, 2024 • edited Loading

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

hellspawn679 commented Jun 18, 2024

ayushrakesh commented Jun 18, 2024

ayushrakesh commented Jun 18, 2024

yurishkuro commented Jun 18, 2024

ayushrakesh commented Jun 19, 2024

yurishkuro commented Jun 22, 2024

ayushrakesh commented Jun 22, 2024 • edited Loading

yurishkuro commented Jun 22, 2024

ayushrakesh commented Jun 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 25, 2024 • edited Loading

Codecov Report

ayushrakesh commented Jun 30, 2024

Choose a reason for hiding this comment

yurishkuro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yurishkuro commented Aug 5, 2024

ayushrakesh commented Jun 18, 2024 •

edited

Loading

ayushrakesh commented Jun 22, 2024 •

edited

Loading

codecov bot commented Jun 25, 2024 •

edited

Loading