Copilot commented on code in PR #81:
URL:
https://github.com/apache/cloudstack-kubernetes-provider/pull/81#discussion_r2556662972
##########
Dockerfile:
##########
@@ -15,10 +15,13 @@
# specific language governing permissions and limitations
# under the License.
-FROM golang:1.21 as builder
+FROM --platform=$BUILDPLATFORM golang:1.23 AS builder
+ARG BUILDPLATFORM
Review Comment:
The `BUILDPLATFORM` ARG is declared but never used. When using `FROM
--platform=$BUILDPLATFORM`, Docker automatically provides the `BUILDPLATFORM`
argument, so this explicit declaration is unnecessary and should be removed.
```suggestion
```
##########
.github/workflows/build-docker-image.yml:
##########
@@ -57,8 +50,41 @@ jobs:
- name: Set Docker image FULL TAG
run: echo "FULL_TAG=$(if [ "${{ secrets.DOCKER_REGISTRY }}" = ""
];then echo ${DOCKER_REPOSITORY}/cloudstack-kubernetes-provider:${TAG};else
echo ${{ secrets.DOCKER_REGISTRY
}}/${DOCKER_REPOSITORY}/cloudstack-kubernetes-provider:${TAG};fi)" >>
$GITHUB_ENV
- - name: Build the Docker image for cloudstack-kubernetes-provider
- run: docker build . --file Dockerfile --tag ${FULL_TAG}
+ - name: Check if should push
+ id: should_push
+ run: |
+ if [ "${{ github.event_name }}" != "pull_request" ] || [ "${{
github.event.pull_request.head.repo.full_name }}" = "${{ github.repository }}"
]; then
+ echo "should_push=true" >> $GITHUB_OUTPUT
+ else
+ echo "should_push=false" >> $GITHUB_OUTPUT
+ fi
- - name: Push Docker image to Docker Registry
- run: docker push ${FULL_TAG}
+ - name: Set up Docker Buildx
+ uses: docker/setup-buildx-action@v3
+
+ - name: Login to Docker Registry
+ if: steps.should_push.outputs.should_push == 'true'
+ uses: docker/login-action@v3
+ with:
+ registry: ${{ secrets.DOCKER_REGISTRY }}
+ username: ${{ secrets.DOCKERHUB_USER }}
+ password: ${{ secrets.DOCKERHUB_TOKEN }}
+
+ - name: Set cache configuration
+ id: cache_config
+ run: |
+ if [ "${{ steps.should_push.outputs.should_push }}" = "true" ]; then
+ echo "cache_from=type=registry,ref=${FULL_TAG}-cache" >>
$GITHUB_OUTPUT
+ echo "cache_to=type=registry,ref=${FULL_TAG}-cache,mode=max" >>
$GITHUB_OUTPUT
+ fi
+
+ - name: Build and push Docker image for cloudstack-kubernetes-provider
(multi-arch)
+ uses: docker/build-push-action@v6
+ with:
+ context: .
+ file: ./Dockerfile
+ platforms: linux/amd64,linux/arm64
+ push: ${{ steps.should_push.outputs.should_push == 'true' }}
+ tags: ${{ env.FULL_TAG }}
+ cache-from: ${{ steps.cache_config.outputs.cache_from }}
+ cache-to: ${{ steps.cache_config.outputs.cache_to }}
Review Comment:
The cache configuration step only sets outputs when `should_push` is true,
but lines 89-90 unconditionally reference these outputs. When building PRs from
forks (where `should_push=false`), these will be empty strings, which may cause
the build action to fail or behave unexpectedly. Consider setting default empty
values or making the cache-from/cache-to parameters conditional in the
build-push-action.
```suggestion
- name: Build and push Docker image for cloudstack-kubernetes-provider
(multi-arch) [trusted]
if: steps.should_push.outputs.should_push == 'true'
uses: docker/build-push-action@v6
with:
context: .
file: ./Dockerfile
platforms: linux/amd64,linux/arm64
push: true
tags: ${{ env.FULL_TAG }}
cache-from: ${{ steps.cache_config.outputs.cache_from }}
cache-to: ${{ steps.cache_config.outputs.cache_to }}
- name: Build Docker image for cloudstack-kubernetes-provider
(multi-arch) [untrusted]
if: steps.should_push.outputs.should_push != 'true'
uses: docker/build-push-action@v6
with:
context: .
file: ./Dockerfile
platforms: linux/amd64,linux/arm64
push: false
tags: ${{ env.FULL_TAG }}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
