Skip to content

Conversation

@alookik27
Copy link

@alookik27 alookik27 commented Apr 1, 2025

Checklist

Reference to Existing Issue

Closes #74 .

Description of Changes

Fortunately i have just edit the ci.yml file ,and add the SHA-256 checksum before uploading the packages to downloads.openwisp.io .

Fortunately i have just edit the ci.yml file ,and add the SHA-256 checksum before uploading the packages to downloads.openwisp.io .

Fixes  openwisp#74
Fortunately i have just edit the ci.yml file ,and add the SHA-256 checksum before uploading the packages to downloads.openwisp.io
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Can you please show a visual result of how the user can find out about the checksum?
And show the generated output?

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

As discussed during the weekly call, the compilation steps generates a file which contains the checksum.
I recommend compiling locally, find the file which contains the checksum and publish it on Google Cloud as "Packages.sha256.checksum".

Comment on lines +114 to +118
- name: Define Storage Paths
run: |
echo "SRC_URL=${{ env.DOWNLOADS_DIR }}/${{ env.START_TIME }}/openwisp_monitoring" >> $GITHUB_ENV
echo "DST_URL=gs://${{ secrets.GCS_DOWNLOADS_BUCKET_NAME }}/openwisp-monitoring/${{ env.START_TIME }}-${{ env.COMMIT_SHA }}" >> $GITHUB_ENV
echo "LATEST_URL=gs://${{ secrets.GCS_DOWNLOADS_BUCKET_NAME }}/openwisp-monitoring/latest" >> $GITHUB_ENV
Copy link
Member

Choose a reason for hiding this comment

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

Why this change was required? We are already defining env in the next command where they are needed

Copy link
Author

Choose a reason for hiding this comment

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

Sorry to answer you so late ,i have reason besides that
the reason for defining the env variable "Define Storage Paths" is for readability ,reuse because the paths are bit long and used in many commands i think it will be simple for maintainability

i edited the file while learning yaml so i searched and found out that adding the env variable can be good for reuse
if these change are bad then i would remove them and edit the file .

Copy link
Member

Choose a reason for hiding this comment

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

It would have been good if these env variables were used anywhere else. These variables are specific for Google Cloud only and therefore added there.

I don't think moving them upwards solves any for our use case.

Comment on lines +123 to +126
gsutil -m rsync -r $SRC_URL $DST_URL
gsutil -m rsync -r -d $SRC_URL $LATEST_URL
gsutil cp $SRC_URL/sha256sums.txt $DST_URL/
gsutil cp $SRC_URL/sha256sums.txt $LATEST_URL/
Copy link
Member

Choose a reason for hiding this comment

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

If we are already syncing the source with destination, then why we need to add checksum files ourselves? Shouldn't they will be added automatically in the bucket if they are available?

Copy link
Author

Choose a reason for hiding this comment

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

for the safety reason if anything happens than the verification can be smoothly done by checksum file.

Copy link
Member

Choose a reason for hiding this comment

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

Do you mean that in any case the packages are not uploaded online, then we can check it via Checksum? I am not very much convinced.

What do you think @nemesifier ?

@devkapilbansal
Copy link
Member

Can you please show a visual result of how the user can find out about the checksum? And show the generated output?

+1

@devkapilbansal
Copy link
Member

@alookik27 Ping!

Comment on lines +114 to +118
- name: Define Storage Paths
run: |
echo "SRC_URL=${{ env.DOWNLOADS_DIR }}/${{ env.START_TIME }}/openwisp_monitoring" >> $GITHUB_ENV
echo "DST_URL=gs://${{ secrets.GCS_DOWNLOADS_BUCKET_NAME }}/openwisp-monitoring/${{ env.START_TIME }}-${{ env.COMMIT_SHA }}" >> $GITHUB_ENV
echo "LATEST_URL=gs://${{ secrets.GCS_DOWNLOADS_BUCKET_NAME }}/openwisp-monitoring/latest" >> $GITHUB_ENV
Copy link
Member

Choose a reason for hiding this comment

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

It would have been good if these env variables were used anywhere else. These variables are specific for Google Cloud only and therefore added there.

I don't think moving them upwards solves any for our use case.

Comment on lines +123 to +126
gsutil -m rsync -r $SRC_URL $DST_URL
gsutil -m rsync -r -d $SRC_URL $LATEST_URL
gsutil cp $SRC_URL/sha256sums.txt $DST_URL/
gsutil cp $SRC_URL/sha256sums.txt $LATEST_URL/
Copy link
Member

Choose a reason for hiding this comment

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

Do you mean that in any case the packages are not uploaded online, then we can check it via Checksum? I am not very much convinced.

What do you think @nemesifier ?

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.

[ci] Upload checksum value for packages

3 participants