search

IBM / ubiquity

Ubiquity
Apache License 2.0
90 stars 26 forks source link

Add image verification for ubiquity and ubiquity-db #188

Closed danare closed 6 years ago

danare commented 6 years ago

This change is Reviewable

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 54.801% when pulling dd34f78d688166157757ae710a476d56637151d5 on dana_images_verification into c94b05f968d11d7b349b83def81d3bc1a87c297a on dev.

shay-berman commented 6 years ago

Review status: 0 of 6 files reviewed at latest revision, 5 unresolved discussions.

scripts/images_verification/ubiquity_db_image_verification/ubiquity_db_image_command_test.yaml, line 5 at r1 (raw file):

commandTests:
# check that /ubiquity-docker-entrypoint.sh symlink exist
  - name: "/ubiquity-docker-entrypoint.sh symlink exist"

@dana why you don't do this file check inside the ubiqutiy_db_image_file_exist_test.yaml file? I think it should be there, and you can use the fileExistenceTest which are better the regular expression of the ls command.

scripts/images_verification/ubiquity_db_image_verification/ubiquity_db_image_file_exist_test.yaml, line 31 at r1 (raw file):

    path: '/NOTICES'
    shouldExist: true
    permissions: '-rw-r--r--'

can you add another check to verify that the file is not in Zero size? it could be good as well.

scripts/images_verification/ubiquity_image_verification/ubiquity_image_command_test.yaml, line 3 at r1 (raw file):

schemaVersion: '2.0.0'

commandTests:

like!

if there is a way to check that the base image is Alpine, it will also be good.

scripts/images_verification/ubiquity_image_verification/ubiquity_image_file_exist_test.yaml, line 7 at r1 (raw file):

    path: '/root/ubiquity'
    shouldExist: true
    permissions: '-rwxr-xr-x'

please also add file owner ship (and as i mention, please add check size of the file is not Zero)

scripts/images_verification/ubiquity_image_verification/ubiquity_image_metadata_test.yaml, line 7 at r1 (raw file):

      value: "/root:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
    - key: "UBIQUITY_SERVER_CERT_PRIVATE"
      value: "/var/lib/ubiquity/ssl/private/ubiquity.key"

nice!

Comments from Reviewable

shay-berman commented 6 years ago

Reviewed 6 of 6 files at r1. Review status: all files reviewed at latest revision, 5 unresolved discussions.

scripts/images_verification/ubiquity_db_image_verification/ubiquity_db_image_command_test.yaml, line 5 at r1 (raw file):

Previously, shay-berman wrote…
@dana why you don't do this file check inside the ubiqutiy_db_image_file_exist_test.yaml file? I think it should be there, and you can use the fileExistenceTest which are better the regular expression of the ls command.

After discussion, LGTM

scripts/images_verification/ubiquity_db_image_verification/ubiquity_db_image_file_exist_test.yaml, line 31 at r1 (raw file):

Previously, shay-berman wrote…
can you add another check to verify that the file is not in Zero size? it could be good as well.

After discussion, you will do it later on by using commands (since there are no directive for that in the tool)

scripts/images_verification/ubiquity_image_verification/ubiquity_image_file_exist_test.yaml, line 7 at r1 (raw file):

Previously, shay-berman wrote…
please also add file owner ship (and as i mention, please add check size of the file is not Zero)

After discussion, you will do it later on by using commands (since there are no directive for that in the tool)

Comments from Reviewable

shay-berman commented 6 years ago
:lgtm:

Review status: all files reviewed at latest revision, 5 unresolved discussions.

Comments from Reviewable