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

Remove default creds #138

Closed
wants to merge 1 commit into from
Closed

Remove default creds #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/.env
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123!
Copy link
Collaborator

Choose a reason for hiding this comment

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

@derek-ho can you directly the env value in docker-compose file?

6 changes: 3 additions & 3 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ jobs:
key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
restore-keys: ${{ runner.os }}-go-
- name: Run Docker containers
run: docker-compose up --detach
run: docker-compose --env-file .github/workflows/.env up --detach
env:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@derek-ho @DarshitChanpura you can directly update here https://github.com/opensearch-project/terraform-provider-opensearch/blob/main/docker-compose.yml, no need for a new .github/workflows/.env. Also the main branch works for past releases of 2.x series, where its not required to add OPENSEARCH_INITIAL_ADMIN_PASSWORD. AFAIK it wont break but OPENSEARCH_INITIAL_ADMIN_PASSWORD would just redundant for old versions right?

Copy link
Member

Choose a reason for hiding this comment

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

agreed. no need to modify this command. we can modify the docker-compose.yml directly if that's the file being used here to spin-up containers.

AFAIK it wont break but OPENSEARCH_INITIAL_ADMIN_PASSWORD would just redundant for old versions right?

Correct. This variable will only be used for 2.12 and above. Else it would simply not be used.

OSS_IMAGE: "${{ matrix.oss-image }}:${{ matrix.version }}"
OS_COMMAND: "${{matrix.OS_COMMAND}}"
Expand All @@ -91,13 +91,13 @@ jobs:
- name: Wait for Opensearch
# ensure that OS has come up and is available
run: |
./script/wait-for-endpoint --timeout=20 http://admin:admin@localhost:9200
./script/wait-for-endpoint --timeout=20 http://admin:myStrongPassword123!@localhost:9200
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will fail for versions below 2.12.0.

- name: Dump docker logs on failure
if: failure()
uses: jwalton/gh-docker-logs@v2
- name: Run the tests
run: |
export OPENSEARCH_URL=http://admin:admin@localhost:9200
export OPENSEARCH_URL=http://admin:myStrongPassword123!@localhost:9200
Copy link
Member

Choose a reason for hiding this comment

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

an alternative approach: we can define an env variable for this action and use that.

env:
    ADMIN_PASSWORD: myStrongPassword123!

and use it as ${{ env.ADMIN_PASSWORD }}:

export OPENSEARCH_URL=http://admin:${{ env.ADMIN_PASSWORD }@localhost:9200

export TF_LOG=INFO
TF_ACC=1 go test ./... -v -parallel 20 -cover -short
# check goreleaser config for deprecations
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Examples of resources can be found in the examples directory.
export OSS_IMAGE="opensearchproject/opensearch:2"
docker-compose up -d
docker-compose ps -a # Checks that the process is running
export OPENSEARCH_URL=http://admin:admin@localhost:9200
export OPENSEARCH_URL=http://admin:myStrongPassword123!@localhost:9200
export TF_LOG=INFO
TF_ACC=1 go test ./... -v -parallel 20 -cover -short
```
Expand Down
2 changes: 1 addition & 1 deletion provider/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func init() {

opendistroOriginalConfigureFunc := testAccOpendistroProvider.ConfigureContextFunc
testAccOpendistroProvider.ConfigureContextFunc = func(c context.Context, d *schema.ResourceData) (interface{}, diag.Diagnostics) {
err := d.Set("url", "http://admin:admin@127.0.0.1:9200")
err := d.Set("url", "http://admin:myStrongPassword123!@127.0.0.1:9200")
Copy link
Author

Choose a reason for hiding this comment

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

Not sure if I should modify this/not sure how it's used - maybe in the integration test with opensearch 2.x?

Copy link
Member

Choose a reason for hiding this comment

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

we can fetch this value from os if the tests are run in the docker container spun-up by the test workflow:
os.Getenv("OPENSEARCH_INITIAL_ADMIN_PASSWORD")

@prudhvigodithi correct me if I'm wrong here

if err != nil {
return nil, diag.FromErr(err)
}
Expand Down
Loading