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

[ANE-1018] Fails for remote-dep when it goes above length of 255 #1216

Merged
merged 7 commits into from
Jun 9, 2023

Conversation

meghfossa
Copy link
Contributor

@meghfossa meghfossa commented Jun 6, 2023

Overview

This PR,

  • Adds remote dep constraint on character length (only in upload mode)

Acceptance criteria

  • Fatally fails, when remote-dep's length is greater than allowed length.

Testing plan

Prereq:

git checkout master && git pull origin && git checkout fix/limit-chars-remote-dep && make install-dev

Test Happy Path:

# > cat fossa-deps.yml
remote-dependencies:
  - name: fossa-cli
    url: https://github.com/fossas/fossa-cli/archive/refs/heads/master.zip
    version: "latest"

Run: fossa analyze -p ane1018 -r happy --fossa-api-key some-key (should succeed)

Test not so Happy Path:

# > cat fossa-deps.yml
remote-dependencies:
  - name: fossa-cli
    url: https://github.com/fossas/fossa-cli/archive/refs/heads/master.zip
    version: "latest"

  - name: long-dep
    url: https://ohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurlohsuchalongurl.com/asset.zip
    version: "0.0.1"

Run: fossa analyze -p ane1018 -r not-happy --fossa-api-key some-key (should fail for fossa-deps.yml)

ane1018-ex

Risks

We only fail when we are not in --output mode, since we don't know the organization Id at output only mode.

Endpoint Code:

Long term, we should have have more holistic validation approach, ideally done at endpoint for things that requires some interaction w endpoint.

Metrics

N/A

References

https://fossa.atlassian.net/browse/ANE-1018

Checklist

  • I added tests for this PR's change (or explained in the PR description why tests don't make sense).
  • If this PR introduced a user-visible change, I added documentation into docs/.
  • If this change is externally visible, I updated Changelog.md. If this PR did not mark a release, I added my changes into an # Unreleased section at the top.
  • If I made changes to .fossa.yml or fossa-deps.{json.yml}, I updated docs/references/files/*.schema.json. You may also need to update these if you have added/removed new dependency type (e.g. pip) or analysis target type (e.g. poetry).

@meghfossa meghfossa requested a review from a team as a code owner June 6, 2023 20:40
@meghfossa meghfossa requested a review from zlav June 6, 2023 20:40
@jssblck
Copy link
Member

jssblck commented Jun 8, 2023

I'm fine with merging this if the backend fix isn't going to be swift, but we have decided to fix this in the backend by simply removing the limit for locator length.

Given that, can we add documentation in the changelog to say something like "this is temporary until the FOSSA backend supports longer locators" so that we don't give customers whiplash?

Update: this is rougher than originally thought, so we'll only revisit if there's significant pain caused by the 255 limit. I retract my comment!

Copy link
Member

@jssblck jssblck left a comment

Choose a reason for hiding this comment

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

Please address comment before merging!

src/App/Fossa/ManualDeps.hs Outdated Show resolved Hide resolved
@meghfossa meghfossa merged commit 3433d58 into master Jun 9, 2023
17 checks passed
@meghfossa meghfossa deleted the fix/limit-chars-remote-dep branch June 9, 2023 18:54
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

2 participants