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

Link Detection Stack Depth FailSafe #159

Merged
merged 15 commits into from
Feb 22, 2023
Merged

Conversation

spiffcs
Copy link
Contributor

@spiffcs spiffcs commented Feb 21, 2023

Summary Follow up to #158

We want to enable a max stack depth so that if the given condition fails to catch any edge cases in #158 stereoscope can still exit safely and report an error.

Tradeoffs

This does leave us open to large stack depth resolutions that are > MaxLinkDepth, but given those cases we can always bump the number to allow for images with large calls to resolveAncestorLinks

spiffcs and others added 9 commits February 21, 2023 11:04
Signed-off-by: Christopher Phillips <[email protected]>
Signed-off-by: Christopher Phillips <[email protected]>
Signed-off-by: Alex Goodman <[email protected]>
Signed-off-by: Alex Goodman <[email protected]>
Signed-off-by: Alex Goodman <[email protected]>
Signed-off-by: Christopher Phillips <[email protected]>
Signed-off-by: Christopher Phillips <[email protected]>
@github-actions
Copy link

github-actions bot commented Feb 21, 2023

Benchmark Test Results

Benchmark results from the latest changes vs base branch
latest: Pulling from library/ubuntu
goos: linux
goarch: amd64
pkg: github.com/anchore/stereoscope/pkg/file
cpu: Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz
docker: 
           │ ./.tmp/benchmark-3c3d250.txt │
           │            sec/op            │
TarIndex-2                   43.71µ ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

           │ ./.tmp/benchmark-3c3d250.txt │
           │             B/op             │
TarIndex-2                  5.560Ki ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

           │ ./.tmp/benchmark-3c3d250.txt │
           │          allocs/op           │
TarIndex-2                    93.00 ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

pkg: github.com/anchore/stereoscope/test/integration
                                      │ ./.tmp/benchmark-3c3d250.txt │
                                      │            sec/op            │
SimpleImage_GetImage/docker-archive-2                   1.526m ± ∞ ¹
SimpleImage_GetImage/oci-archive-2                      1.525m ± ∞ ¹
SimpleImage_GetImage/oci-dir-2                          725.0µ ± ∞ ¹
geomean                                                 1.191m
¹ need >= 6 samples for confidence interval at level 0.95

                                      │ ./.tmp/benchmark-3c3d250.txt │
                                      │             B/op             │
SimpleImage_GetImage/docker-archive-2                  355.5Ki ± ∞ ¹
SimpleImage_GetImage/oci-archive-2                     632.5Ki ± ∞ ¹
SimpleImage_GetImage/oci-dir-2                         399.8Ki ± ∞ ¹
geomean                                                448.0Ki
¹ need >= 6 samples for confidence interval at level 0.95

                                      │ ./.tmp/benchmark-3c3d250.txt │
                                      │          allocs/op           │
SimpleImage_GetImage/docker-archive-2                   2.771k ± ∞ ¹
SimpleImage_GetImage/oci-archive-2                      1.550k ± ∞ ¹
SimpleImage_GetImage/oci-dir-2                          1.334k ± ∞ ¹
geomean                                                 1.789k
¹ need >= 6 samples for confidence interval at level 0.95

docker: Error response from daemon: Get "http://localhost/v2/": dial tcp [::1]:80: connect: connection refused.
                                                   │ ./.tmp/benchmark-3c3d250.txt │
                                                   │            sec/op            │
SimpleImage_FetchSquashedContents/docker-archive-2                   17.85µ ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

                                                   │ ./.tmp/benchmark-3c3d250.txt │
                                                   │             B/op             │
SimpleImage_FetchSquashedContents/docker-archive-2                  2.648Ki ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

                                                   │ ./.tmp/benchmark-3c3d250.txt │
                                                   │          allocs/op           │
SimpleImage_FetchSquashedContents/docker-archive-2                    21.00 ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

@spiffcs spiffcs marked this pull request as ready for review February 22, 2023 15:17
@spiffcs spiffcs marked this pull request as draft February 22, 2023 15:17
@spiffcs spiffcs changed the title Link Detection FailSafe Link Detection Stack Depth FailSafe Feb 22, 2023
@spiffcs spiffcs marked this pull request as ready for review February 22, 2023 18:50
@wagoodman wagoodman enabled auto-merge (squash) February 22, 2023 18:54
@wagoodman wagoodman merged commit fab1c96 into main Feb 22, 2023
@wagoodman wagoodman deleted the sym-link-detection-cycle branch February 22, 2023 18:59
gnmahanth pushed a commit to deepfence/stereoscope that referenced this pull request Jun 15, 2023
* test: add failing test for cycle case

Signed-off-by: Christopher Phillips <[email protected]>

* test: test updates sym links

Signed-off-by: Christopher Phillips <[email protected]>

* change the filetree recursive pathset to represent open calls

Signed-off-by: Alex Goodman <[email protected]>

* add another cycle test

Signed-off-by: Alex Goodman <[email protected]>

* change filetree attempting path set to counters

Signed-off-by: Alex Goodman <[email protected]>

* remove comment

Signed-off-by: Alex Goodman <[email protected]>

* fix linting

Signed-off-by: Alex Goodman <[email protected]>

* feat: decrement stack depth

Signed-off-by: Christopher Phillips <[email protected]>

* test: remove old wip test name

Signed-off-by: Christopher Phillips <[email protected]>

* chore: style updates

Signed-off-by: Christopher Phillips <[email protected]>

* feat: move maxLinkDepth decrement to inside ancestor loop

Signed-off-by: Christopher Phillips <[email protected]>

* feat: move maxLinkDepth decrement into resolveNodeLinks loop

Signed-off-by: Christopher Phillips <[email protected]>

* feat: move detection to top and write minimal test case

Signed-off-by: Christopher Phillips <[email protected]>

* test: update linkResolution test to use internal value

Signed-off-by: Christopher Phillips <[email protected]>

---------

Signed-off-by: Christopher Phillips <[email protected]>
Signed-off-by: Alex Goodman <[email protected]>
Co-authored-by: Alex Goodman <[email protected]>
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.

2 participants