ansible / ansible-builder

An Ansible execution environment builder
Other
287 stars 93 forks source link

fix: capitalize 'as' to follow from-as-casing rule #687

Closed kurokobo closed 2 months ago

kurokobo commented 2 months ago

Closes #686

This PR capitalizes as to follow the from-as-casing rule to supress warnings during build on Docker Engine 27+: https://docs.docker.com/reference/build-checks/from-as-casing/

Test:

# Install customized Ansible Builder
pip install .
# Ensure no warnings are displayed during build
$ ansible-builder build --tag registry.example.com/ansible/ee:2.17-minimal --container-runtime docker --verbosity 3
Ansible Builder is generating your execution environment build context.
...
Ansible Builder is building your execution environment image. Tags: registry.example.com/ansible/ee:2.17-minimal
Running command:
  docker build -f context/Dockerfile -t registry.example.com/ansible/ee:2.17-minimal context
#0 building with "default" instance using docker driver

#1 [internal] load build definition from Dockerfile
#1 transferring dockerfile: 2.96kB done
#1 DONE 0.0s
...
#32 exporting to image
#32 exporting layers done
#32 writing image sha256:73227f703dbf35eb822caa325f0db8593fb10719bbb64d21615ea7ddd50df04e done
#32 naming to registry.example.com/ansible/ee:2.17-minimal done
#32 DONE 0.0s

Complete! The build context can be found at: /home/********/builder/context
# Ensure `AS` in the generated Dockerfile are capitalized
$ cat context/Dockerfile 
...
# Base build stage
FROM $EE_BASE_IMAGE AS base
...
# Galaxy build stage
FROM base AS galaxy
...
# Builder build stage
FROM base AS builder
...
# Final build stage
FROM base AS final
# Ensure `docker build --check .` passed
$ cd context/
$ docker build --check .
[+] Building 0.7s (3/3) FINISHED                                                                                                                                            docker:default
 => [internal] load build definition from Dockerfile                                                                                                                                  0.0s
 => => transferring dockerfile: 2.96kB                                                                                                                                                0.0s
 => [internal] load metadata for quay.io/centos/centos:stream9-minimal                                                                                                                0.7s
 => [internal] load .dockerignore                                                                                                                                                     0.0s
 => => transferring context: 2B  
kurokobo commented 2 months ago

Oh, I missed that there are unit tests. Will update this PR to fix tests.

kurokobo commented 2 months ago

Updated.

Shrews commented 2 months ago

Assuming these pass tests, I'll be ok with this change. But I don't want to really begin chasing docker linting rules, at least not without some sort of testing in place within our CI to catch the warnings, and prevent accidental reverts.