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

Deprecate config.yaml as valid config source; Add unit regression for correct config paths #1640

Merged
merged 9 commits into from
Mar 20, 2023

Conversation

AidanDelaney
Copy link
Contributor

@AidanDelaney AidanDelaney commented Mar 2, 2023

Only add the pwd to the config search path if and only if it contains a config file that we expect. This avoids incorrectly finding config files that may be specific to applictions other than syft.

partially addresses #1634

Only add the pwd to the config search path if and only if it contains
a config file that we expect.  This avoids incorrectly finding config
files that may be specific to applictions other than syft.

fixes: anchore#1634

Signed-off-by: Aidan Delaney <adelaney21@bloomberg.net>
@spiffcs
Copy link
Contributor

spiffcs commented Mar 16, 2023

👋 Thanks for the PR @AidanDelaney - do you mind if I make a quick update the keeps the current behavior, but adds a warning.

We're looking to release syft v1.0.0 in the future and think this is a good candidate for that handover.

I don't want to merge this PR as is and break setups for people who are relying on discovering config.yaml.

Instead, if we introduce your changes, but still allow v.AddConfigPath(".") with a logged warning we can communicate to people this will be sunset in the v1.0.0 release

I'll also make sure to add your issue to our v1 milestone so we don't miss removing the warning and sunsetting the wrong behavior

@AidanDelaney
Copy link
Contributor Author

Please feel free to make any update you'd like. I think that adding v.AddConfigPath(".") will mean that this PR then does not fix #1634. Do we want to keep that issue open until v1.0.0?

@spiffcs
Copy link
Contributor

spiffcs commented Mar 20, 2023

Yep! We'll keep it open - I added it to our milestone tracking for v1.0.0 - I'll push the warning change now and merge this keeping the old behavior, but with the notice of deprecation and intent to fix by v1.0.0

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs changed the title Add pwd to config search path iff it contains a config Add pwd to config search path if it contains a config Mar 20, 2023
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs changed the title Add pwd to config search path if it contains a config Deprecate config.yaml as valid config source; Add unit regression for correct config paths Mar 20, 2023
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Copy link
Contributor

@kzantow kzantow left a comment

Choose a reason for hiding this comment

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

👍

@spiffcs
Copy link
Contributor

spiffcs commented Mar 20, 2023

Big thanks to @AidanDelaney for surfacing this issue - it's on the board for v1.0.0

@spiffcs spiffcs merged commit f11a7b5 into anchore:main Mar 20, 2023
spiffcs added a commit to deitch/syft that referenced this pull request Mar 21, 2023
* main: (47 commits)
  Deprecate config.yaml as valid config source; Add unit regression for correct config paths (anchore#1640)
  chore: Update syft bootstrap tools to latest versions. (anchore#1682)
  Update documentation: (anchore#1680)
  chore: Update Stereoscope to 7928713c391e20abaede6a029f4ce37b628a4c8b (anchore#1681)
  fix: reduce logging for bad dpkg lines (anchore#1675)
  fix ruby classifier (anchore#1678)
  feat: add shared dir for easier cleanup (anchore#1676)
  chore(deps): bump github.com/google/go-containerregistry (anchore#1672)
  chore(deps): bump actions/setup-go from 3 to 4 (anchore#1671)
  fix: move defer after error to protect panic case (anchore#1670)
  feat: add argocd, helm, kustomize and kubectl binary classifiers (anchore#1663)
  defer closing file (anchore#1668)
  fix: remove author contributing to javascript CPEs (anchore#1669)
  fix: more python matching support (anchore#1667)
  Update syft bootstrap tools to latest versions. (anchore#1666)
  feat: add ruby classifier (anchore#1665)
  Update syft bootstrap tools to latest versions. (anchore#1658)
  fix: improved Python binary detection (anchore#1648)
  fix: suppress some known incorrect vendor candidates for npm CPEs (anchore#1659)
  fix: sanitize SPDX LicenseRefs (anchore#1657)
  ...

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
… correct config paths (anchore#1640)

Warn user of future deprecation of ./config.yaml for v1.0.0 release

---------

Signed-off-by: Aidan Delaney <adelaney21@bloomberg.net>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Co-authored-by: Christopher Phillips <christopher.phillips@anchore.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants