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

fix: Check file format when uploading artifacts #270

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bahaa-ghazal
Copy link

Changelog: Title
Ticket: MEN-7860

@mender-test-bot
Copy link

@bahaa-ghazal, Let me know if you want to start the integration pipeline by mentioning me and the command "start pipeline".


my commands and options

You can trigger a pipeline on multiple prs with:

  • mentioning me and start pipeline --pr mender/127 --pr mender-connect/255

You can start a fast pipeline, disabling full integration tests with:

  • mentioning me and start pipeline --fast

You can trigger GitHub->GitLab branch sync with:

  • mentioning me and sync

You can cherry pick to a given branch or branches with:

  • mentioning me and:
 cherry-pick to:
 * 1.0.x
 * 2.0.x

Changelog: Title
Ticket: MEN-7860
Signed-off-by: Bahaa Aldeen Ghazal <[email protected]>
@mender-test-bot
Copy link

mender-test-bot commented Dec 23, 2024

Merging these commits will result in the following changelog entries:

Changelogs

mender-cli (MEN-7860)

New changes in mender-cli since master:

Bug Fixes
  • Check file format when uploading artifacts
    (MEN-7860)

@bahaa-ghazal bahaa-ghazal requested a review from alfrunes January 20, 2025 12:16
tr := tar.NewReader(artifact)
versionH, err := tr.Next()
if err != nil {
return errors.Wrap(err, "Cannot find artifact")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we adjust the error message a little?
I found the current message quite confusing when I was testing the CLI.

Suggested change
return errors.Wrap(err, "Cannot find artifact")
return errors.Wrap(err, "error parsing artifact")

Comment on lines +300 to +317
tr := tar.NewReader(artifact)
versionH, err := tr.Next()
if err != nil {
return errors.Wrap(err, "Cannot find artifact")
} else if versionH.Name != "version" {
return errors.New("Invalid artifact format")
}
v := struct {
Format string `json:"format"`
}{}
err = json.NewDecoder(tr).Decode(&v)
if err != nil || v.Format != "mender" {
return errors.New("Invalid artifact format")
}
_, err = artifact.Seek(0, io.SeekStart)
if err != nil || v.Format != "mender" {
return err
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: Can you refactor this artifact parsing part into it's own function and reuse it below?
Duplicating code often leads to forgetting about the existence about the other instance when touching the code later.

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.

3 participants