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.

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

Even more scrubbing #5152

Merged
merged 7 commits into from
Apr 26, 2022
Merged

Even more scrubbing #5152

merged 7 commits into from
Apr 26, 2022

Conversation

jtcohen6
Copy link
Contributor

@jtcohen6 jtcohen6 commented Apr 25, 2022

resolves #5151

Description

  • Scrub cmd in CommandError (parent), plus stdout + stderr in CommandResultError (child). The latter is what clients.system.run_cmd raises and clients.git.clone catches
  • Also scrub it when raising, within run_cmd
  • Also scrub it when catching, in clients.git._raise_git_cloning_error... even though it should be scrubbed yet again in dbt.exceptions.bad_package_spec

This is duplicative logic, but so it goes... until #4836, perhaps?

Locally

packages:
  - git: "https://fakeuser:{{ env_var('DBT_ENV_SECRET_GIT_TOKEN') }}@github.com/dbt-labs/fake-repo.git"

Invoking dbt-core as a Python module...

>>> from dbt.main import handle_and_check
>>> handle_and_check(['deps'])
18:33:14  Running with dbt=1.2.0-a1
Traceback (most recent call last):
  File "/Users/jerco/dev/product/dbt-core/core/dbt/clients/git.py", line 65, in clone
    result = run_cmd(cwd, clone_cmd, env={"LC_ALL": "C"})
  File "/Users/jerco/dev/product/dbt-core/core/dbt/clients/system.py", line 435, in run_cmd
    raise dbt.exceptions.CommandResultError(cwd, scrubbed_cmd, proc.returncode, out, err)
dbt.exceptions.CommandResultError: Got a non-zero returncode running: ['/usr/bin/git', 'clone', '--depth', '1', 'https://fakeuser:*****@github.com/dbt-labs/fake-repo.git', '3031f9cd09251a86b0009166b1370c80']

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/jerco/dev/product/dbt-core/core/dbt/main.py", line 191, in handle_and_check
    task, res = run_from_args(parsed)
  File "/Users/jerco/dev/product/dbt-core/core/dbt/main.py", line 238, in run_from_args
    results = task.run()
  File "/Users/jerco/dev/product/dbt-core/core/dbt/task/deps.py", line 56, in run
    final_deps = resolve_packages(packages, self.config)
  File "/Users/jerco/dev/product/dbt-core/core/dbt/deps/resolver.py", line 131, in resolve_packages
    target = final[package].resolved().fetch_metadata(config, renderer)
  File "/Users/jerco/dev/product/dbt-core/core/dbt/deps/base.py", line 86, in fetch_metadata
    self._cached_metadata = self._fetch_metadata(project, renderer)
  File "/Users/jerco/dev/product/dbt-core/core/dbt/deps/git.py", line 93, in _fetch_metadata
    path = self._checkout()
  File "/Users/jerco/dev/product/dbt-core/core/dbt/deps/git.py", line 79, in _checkout
    dir_ = git.clone_and_checkout(
  File "/Users/jerco/dev/product/dbt-core/core/dbt/clients/git.py", line 137, in clone_and_checkout
    _, err = clone(
  File "/Users/jerco/dev/product/dbt-core/core/dbt/clients/git.py", line 67, in clone
    _raise_git_cloning_error(repo, revision, exc)
  File "/Users/jerco/dev/product/dbt-core/core/dbt/clients/git.py", line 37, in _raise_git_cloning_error
    bad_package_spec(repo, revision, stderr)
  File "/Users/jerco/dev/product/dbt-core/core/dbt/exceptions.py", line 709, in bad_package_spec
    raise InternalException(scrub_secrets(msg, env_secrets()))
dbt.exceptions.InternalException: Error checking out spec='None' for repo https://fakeuser:*****@github.com/dbt-labs/fake-repo.git
Cloning into '3031f9cd09251a86b0009166b1370c80'...
remote: Support for password authentication was removed on August 13, 2021. Please use a personal access token instead.
remote: Please see https://github.blog/2020-12-15-token-authentication-requirements-for-git-operations/ for more information.
fatal: Authentication failed for 'https://github.com/dbt-labs/fake-repo.git/'

Checklist

@jtcohen6 jtcohen6 requested a review from a team as a code owner April 25, 2022 18:39
@cla-bot cla-bot bot added the cla:yes label Apr 25, 2022
@jtcohen6
Copy link
Contributor Author

@ChenyuLInx Thanks for picking this up! I'll leave to other reviews for final approval.

After merging, we need to backport to both 1.1.latest + 1.0.latest.

Copy link
Member

@emmyoop emmyoop left a comment

Choose a reason for hiding this comment

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

Just a comment but otherwise looks good.

self.args = (cwd, cmd, returncode, stdout, stderr, message)
self.stdout = scrub_secrets(stdout.decode("utf-8"), env_secrets())
self.stderr = scrub_secrets(stderr.decode("utf-8"), env_secrets())
self.args = (cwd, self.cmd, returncode, stdout, stderr, message)
Copy link
Member

Choose a reason for hiding this comment

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

Do we not need to use the scrubbed stdout/stderr here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch! I wonder what we are using the self.args for

Copy link
Member

@emmyoop emmyoop left a comment

Choose a reason for hiding this comment

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

Look good.

@ChenyuLInx ChenyuLInx merged commit ce0bcc0 into main Apr 26, 2022
@ChenyuLInx ChenyuLInx deleted the fix/more-scrubbing-logic branch April 26, 2022 15:35
@ChenyuLInx ChenyuLInx added backport 1.1.latest backport 1.0.latest Tag for PR to be backported to the 1.0.latest branch labels Apr 26, 2022
github-actions bot pushed a commit that referenced this pull request Apr 26, 2022
* Even more scrubbing

* Changelog entry

* Even more

* remove reduendent scrub

* remove reduendent scrub

* fix encoding issue

* keep scrubbed log in args

Co-authored-by: Chenyu Li <[email protected]>
(cherry picked from commit ce0bcc0)
@github-actions
Copy link
Contributor

The backport to 1.0.latest failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.0.latest 1.0.latest
# Navigate to the new working tree
cd .worktrees/backport-1.0.latest
# Create a new branch
git switch --create backport-5152-to-1.0.latest
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 ce0bcc08a6b4933ff8ae7558f6c8f7722ff03dbe
# Push it to GitHub
git push --set-upstream origin backport-5152-to-1.0.latest
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.0.latest

Then, create a pull request where the base branch is 1.0.latest and the compare/head branch is backport-5152-to-1.0.latest.

ChenyuLInx pushed a commit that referenced this pull request Apr 26, 2022
* Even more scrubbing

* Changelog entry

* Even more

* remove reduendent scrub

* remove reduendent scrub

* fix encoding issue

* keep scrubbed log in args

Co-authored-by: Chenyu Li <[email protected]>
(cherry picked from commit ce0bcc0)
ChenyuLInx added a commit that referenced this pull request Apr 26, 2022
* Even more scrubbing

* Changelog entry

* Even more

* remove reduendent scrub

* remove reduendent scrub

* fix encoding issue

* keep scrubbed log in args

Co-authored-by: Chenyu Li <[email protected]>
(cherry picked from commit ce0bcc0)

Co-authored-by: Jeremy Cohen <[email protected]>
ChenyuLInx pushed a commit that referenced this pull request Apr 26, 2022
* Even more scrubbing

* Changelog entry

* Even more

* remove reduendent scrub

* remove reduendent scrub

* fix encoding issue

* keep scrubbed log in args

Co-authored-by: Chenyu Li <[email protected]>
(cherry picked from commit ce0bcc0)

Co-authored-by: Jeremy Cohen <[email protected]>
agoblet pushed a commit to BigDataRepublic/dbt-core that referenced this pull request May 20, 2022
* Even more scrubbing

* Changelog entry

* Even more

* remove reduendent scrub

* remove reduendent scrub

* fix encoding issue

* keep scrubbed log in args

Co-authored-by: Chenyu Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.0.latest Tag for PR to be backported to the 1.0.latest branch backport 1.1.latest cla:yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-551] Extra scrubbing for git clone exceptions
4 participants