test: prevent secret output #118
Conversation
ptiurin
force-pushed
the
sec-dont-print-secrets
branch
from
950655e
to
330e362
Compare
| - name: Keep environment name in the summary | ||
| run: echo '### Ran integration tests against ${{ inputs.environment }} ' >> $GITHUB_STEP_SUMMARY | ||
|
|
||
| - name: Setup database and engine | ||
| id: setup | ||
| uses: firebolt-db/integration-testing-setup@v1 | ||
| with: | ||
| firebolt-username: ${{ env.USERNAME }} | ||
| firebolt-password: ${{ env.PASSWORD }} | ||
| firebolt-username: ${{ inputs.environment == 'staging' && secrets.FIREBOLT_STG_USERNAME || secrets.FIREBOLT_USERNAME_DEV }} |
There was a problem hiding this comment.
Does this change anything?
There was a problem hiding this comment.
Yes, special characters were bugged in the previous implementation - https://github.com/firebolt-db/dbt-firebolt/pull/118/files#diff-29ce8407d15551c9c62184e207b6ceadebb261f7ffbfb75d7d67844ba33709abL51
if you had a $ in the password for example you'd have bash resolving that variable to an empty string therefore rendering the password unusable. This implementation does not pass around secrets but uses the correct one instead so it's kept as is.
There was a problem hiding this comment.
This is probably a bug in every repo. Yet, since we are removing dev support this will be fixed by it's own, so nothing to do I guess
tests/conftest.py
Outdated
|
|
||
| def represent_data(self, data): | ||
| if isinstance(data, Secret): | ||
| return self.represent_scalar('tag:yaml.org,2002:str', data.value) |
There was a problem hiding this comment.
Please add a comment to explain this formatting. What would be the resulting example for a secret here?
There was a problem hiding this comment.
Found a better function to use and also added a comment.
There was a problem hiding this comment.
Here we dump the actual secret into the profiles.yml config file for DBT. Everywhere else it would be obscured.
|
FIR-29705
Preventing accidental secret output.
Changing staging/dev secrets logic since the old one didn't handle special characters correctly and now we have those with the new credentials. In any case, dev will be retired soon.