Skip to content

Commit

Permalink
Fix build workflow from forks (radius-project#6544)
Browse files Browse the repository at this point in the history
# Description

This change attempts to fix the build/test workflow when initiated from
a fork.

The following problems are being addressed in this change:

- The build/push container workflow will fail because the fork cannot
push to OUR ghcr repo.
- The build/push helm workflow will fail because the fork cannot push to
OUR acr or ghcr repo.
- The Bicep linting workflow cannot clone the docs or samples repo
because its missing a github tokein.

The following changes are being made here:

- Move Helm linting to our 'lint' workflow. This can run without
dependencies, and it's easier to combine it with the other linting we
do.
- Add new target for multi-arch container build so we can test the build
without doing a push.
- Use the new multi-arch container build target when a PR is running.
- This is OK because the functional-test workflow does its own
build/push of containers.
- We want to still test that the multi-arch build succeeds on PR, but we
don't need to push the result anywhere.
- Skip the helm job when a PR is running.
- This is OK because the functional-test workflow uses the chart from
the local directory.
- We want to still test the Helm linter, which is why it was moved
elsewhere.
- Remove the 'clone' of the docs and samples repo as part of the
validate-bicep workflow.
- This is OK because this was never a good idea. Trying to do multi-repo
validation like this is going to lead to a lot of false positives.

## Type of change

<!--

Please select **one** of the following options that describes your
change and delete the others. Clearly identifying the type of change you
are making will help us review your PR faster, and is used in authoring
release notes.

If you are making a bug fix or functionality change to Radius and do not
have an associated issue link please create one now.

-->

- This pull request is a minor refactor, code cleanup, test improvement,
or other maintenance task and doesn't change the functionality of Radius
(issue link optional).

<!--

Please update the following to link the associated issue. This is
required for some kinds of changes (see above).

-->

Fixes: #issue_number

## Auto-generated summary

<!--
GitHub Copilot for docs will auto-generate a summary of the PR
-->

<!--
copilot:all
-->
### <samp>🤖 Generated by Copilot at ec2c934</samp>

### Summary
🚀🧹🛠️

<!--
1.  🚀 for removing unnecessary steps and speeding up the workflows
2.  🧹 for simplifying and cleaning up the logic and code
3.  🛠️ for fixing the helm linting issue and improving the build process
-->
This pull request simplifies and optimizes the GitHub workflows for
building, linting, and validating the radius project. It removes
unnecessary steps and dependencies, and consolidates the helm chart
linting into the build workflow. It also refactors the docker multi-arch
build process into separate targets.

> _Some workflows were changed in this PR_
> _To make them more efficient by far_
> _`lint.yaml` lost `Azure CLI`_
> _`validate-bicep` got simplified_
> _And `build.yaml` got a new star_

### Walkthrough
* Simplify and secure the container image building and pushing process
([link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-d0777657fa3fd81d23aaf7273e58aee453b04e67882517900c56daeef9b3e4c1L47-R47),
[link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-d0777657fa3fd81d23aaf7273e58aee453b04e67882517900c56daeef9b3e4c1L257-R260),
[link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-d0777657fa3fd81d23aaf7273e58aee453b04e67882517900c56daeef9b3e4c1R278-R279),
[link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-d0777657fa3fd81d23aaf7273e58aee453b04e67882517900c56daeef9b3e4c1L283-R287),
[link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-e5017862a52d1de76ba232f290f53785637620580359895b37c593f181afc6c1L58-R78),
[link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-e5017862a52d1de76ba232f290f53785637620580359895b37c593f181afc6c1R120-R122),
[link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-e5017862a52d1de76ba232f290f53785637620580359895b37c593f181afc6c1L122-R146))
- Use a single container registry `ghcr.io/radius-project` for all
images and avoid pushing images from pull requests
([link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-d0777657fa3fd81d23aaf7273e58aee453b04e67882517900c56daeef9b3e4c1L47-R47),
[link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-d0777657fa3fd81d23aaf7273e58aee453b04e67882517900c56daeef9b3e4c1L257-R260),
[link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-d0777657fa3fd81d23aaf7273e58aee453b04e67882517900c56daeef9b3e4c1R278-R279))
- Use a single OCI repository `radius-project/helm-chart` for the helm
chart and version it based on the git ref
([link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-d0777657fa3fd81d23aaf7273e58aee453b04e67882517900c56daeef9b3e4c1L283-R287))
- Split the `generateDockerMultiArches` macro into separate targets for
building and pushing the multi-arch images
([link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-e5017862a52d1de76ba232f290f53785637620580359895b37c593f181afc6c1L58-R78),
[link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-e5017862a52d1de76ba232f290f53785637620580359895b37c593f181afc6c1R120-R122),
[link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-e5017862a52d1de76ba232f290f53785637620580359895b37c593f181afc6c1L122-R146))
- Clarify the comments and variables for the test and non-test image
targets
([link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-e5017862a52d1de76ba232f290f53785637620580359895b37c593f181afc6c1L70-R87),
[link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-e5017862a52d1de76ba232f290f53785637620580359895b37c593f181afc6c1L108-R131))
* Move the helm chart linting from `lint.yaml` to `build.yaml` and
remove the unnecessary Azure CLI setup
([link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-4b122024a3a28ded65da76a2f1bface1f3a27328374438d1298d25585fe0603bL62-R68),
[link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-d0777657fa3fd81d23aaf7273e58aee453b04e67882517900c56daeef9b3e4c1L283-R287))
- Add the `Install helm` and `Run Helm linter` steps to the `build.yaml`
workflow to check the validity and quality of the helm chart
([link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-d0777657fa3fd81d23aaf7273e58aee453b04e67882517900c56daeef9b3e4c1L283-R287))
- Remove the `Setup Azure CLI` step from the `lint.yaml` workflow as it
is not needed for linting
([link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-4b122024a3a28ded65da76a2f1bface1f3a27328374438d1298d25585fe0603bL62-R68))
* Remove the unused steps and repos from the `validate-bicep.yaml`
workflow
([link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-4f41a75886a1a479805bae1b7fb335479735aa7740e02b9e5c19992ead18f73eL36-R46))
- Remove the steps to clone the docs and samples repos and to login to
Azure CLI as they are not needed for validating the bicep files
([link](https://github.com/radius-project/radius/pull/6544/files?diff=unified&w=0#diff-4f41a75886a1a479805bae1b7fb335479735aa7740e02b9e5c19992ead18f73eL36-R46))

Signed-off-by: willdavsmith <[email protected]>
  • Loading branch information
rynowak authored and willdavsmith committed Nov 3, 2023
1 parent 98a8767 commit be1d6a1
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 48 deletions.
15 changes: 9 additions & 6 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ env:
GHCR_ACTOR: rad-ci-bot

# Container registry url for GitHub container registry.
CONTAINER_REGISTRY: ${{ github.event_name == 'pull_request' && 'ghcr.io/radius-project/dev' || 'ghcr.io/radius-project' }}
CONTAINER_REGISTRY: 'ghcr.io/radius-project'

# Local file path to the release binaries.
RELEASE_PATH: ./release
Expand Down Expand Up @@ -254,10 +254,10 @@ jobs:
env:
DOCKER_REGISTRY: ${{ env.CONTAINER_REGISTRY }}
DOCKER_TAG_VERSION: latest
- name: Push container images (PR)
- name: Build container images (PR) # Don't push on PR, agent will not have permission.
run: |
make docker-test-image-build && make docker-test-image-push
make docker-multi-arch-push
make docker-test-image-build
make docker-multi-arch-build
if: github.event_name == 'pull_request'
env:
DOCKER_REGISTRY: ${{ env.CONTAINER_REGISTRY }}
Expand All @@ -275,13 +275,16 @@ jobs:
name: Helm chart build
needs: ['build-and-push-images']
runs-on: ubuntu-latest
# Don't push on PR, agent will not have permission.
if: (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
env:
ARTIFACT_DIR: ./dist/Charts
HELM_PACKAGE_DIR: helm
HELM_CHARTS_DIR: deploy/Chart
OCI_REGISTRY: ghcr.io
# If we are pushing from a tag or from the main branch, push to radius-project/helm-chart. Otherwise, push to radius-project/dev/helm-chart.
OCI_REPOSITORY: ${{ (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main') && 'radius-project/helm-chart' || 'radius-project/dev/helm-chart' }}
# We only push the chart on pushes to main or to a tag. The versioning logic will select the right
# version for us.
OCI_REPOSITORY: 'radius-project/helm-chart'
steps:
- name: Checkout
uses: actions/checkout@v3
Expand Down
10 changes: 8 additions & 2 deletions .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ jobs:
env:
GOVER: '^1.21'
GOPROXY: https://proxy.golang.org
HELM_CHARTS_DIR: deploy/Chart
steps:
- name: Check out repo
uses: actions/checkout@v3
Expand All @@ -59,8 +60,13 @@ jobs:
run: go install github.com/golang/mock/[email protected]
- name: Install controller-gen
run: go install sigs.k8s.io/controller-tools/cmd/[email protected]
- name: Setup Azure CLI
run: curl -sL https://aka.ms/InstallAzureCLIDeb | sudo bash
- name: Install helm
uses: Azure/setup-helm@v3
with:
version: 'v3.11.1'
- name: Run Helm linter
run: |
helm lint ${{ env.HELM_CHARTS_DIR }}
- name: Run linter
uses: golangci/golangci-lint-action@v3
with:
Expand Down
7 changes: 0 additions & 7 deletions .github/workflows/publish-docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@ on:
branches:
- main
- release/*
pull_request:
types: [opened, synchronize, reopened, closed]
branches:
- main
- release/*

jobs:
cli-docs:
name: Generate CLI reference docs
Expand Down Expand Up @@ -58,7 +52,6 @@ jobs:
path: docs/docs/content/reference/cli
- name: Create pull request
uses: peter-evans/create-pull-request@v5
if: github.event_name != 'pull_request'
with:
token: ${{ secrets.GH_RAD_CI_BOT_PAT }}
path: docs
Expand Down
33 changes: 5 additions & 28 deletions .github/workflows/validate-bicep.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,37 +33,14 @@ jobs:
steps:
- name: Check out repo
uses: actions/checkout@v3

- name: Clone docs repo
uses: actions/checkout@v3
with:
token: ${{ secrets.GH_RAD_CI_BOT_PAT }}
repository: radius-project/docs
path: temp/radius-project/docs
ref: edge
- name: Clone samples repo
uses: actions/checkout@v3
with:
token: ${{ secrets.GH_RAD_CI_BOT_PAT }}
repository: radius-project/samples
path: temp/radius-project/samples
ref: edge
- name: Setup Azure CLI
run: curl -sL https://aka.ms/InstallAzureCLIDeb | sudo bash
- name: az CLI login
run: |
az login --service-principal \
--username ${{ secrets.AZURE_SP_TESTS_APPID }} \
--password ${{ secrets.AZURE_SP_TESTS_PASSWORD }} \
--tenant ${{ secrets.AZURE_SP_TESTS_TENANTID }}
- name: Parse release version and set environment variables
run: python ./.github/scripts/get_release_version.py
- name: Download rad-bicep-corerp
- name: Download rad-bicep
run: |
./.github/scripts/curl-with-retries.sh https://get.radapp.dev/tools/bicep-extensibility/${{ env.REL_CHANNEL }}/linux-x64/rad-bicep --output rad-bicep-corerp
chmod +x rad-bicep-corerp
./rad-bicep-corerp --version
./.github/scripts/curl-with-retries.sh https://get.radapp.dev/tools/bicep-extensibility/${{ env.REL_CHANNEL }}/linux-x64/rad-bicep --output rad-bicep
chmod +x rad-bicep
./rad-bicep --version
- name: Verify Bicep files
run: ./build/validate-bicep.sh
env:
BICEP_PATH: './rad-bicep-corerp'
BICEP_PATH: './rad-bicep'
33 changes: 28 additions & 5 deletions build/docker.mk
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,36 @@ docker-push-$(1):
endef

define generateDockerMultiArches
.PHONY: docker-multi-arch-build-$(1)
docker-multi-arch-build-$(1): build-$(1)-linux-arm64 build-$(1)-linux-amd64 build-$(1)-linux-arm
@echo "$(ARROW) Building Go image $(DOCKER_REGISTRY)/$(1):$(DOCKER_TAG_VERSION)"
@cp -v $(3) $(OUT_DIR)/Dockerfile-$(1)

cd $(OUT_DIR) && docker buildx build -f ./Dockerfile-$(1) \
--platform linux/amd64,linux/arm64,linux/arm \
-t $(DOCKER_REGISTRY)/$(1):$(DOCKER_TAG_VERSION) \
--label org.opencontainers.image.source="$(IMAGE_SRC)" \
--label org.opencontainers.image.description="$(1)" \
--label org.opencontainers.image.version="$(REL_VERSION)" \
--label org.opencontainers.image.revision="$(GIT_COMMIT)" \
$(2)

.PHONY: docker-multi-arch-push-$(1)
docker-multi-arch-push-$(1): build-$(1)-linux-arm64 build-$(1)-linux-amd64 build-$(1)-linux-arm
@echo "$(ARROW) Building Go image $(DOCKER_REGISTRY)/$(1):$(DOCKER_TAG_VERSION)"
@echo "$(ARROW) Building and pushing Go image $(DOCKER_REGISTRY)/$(1):$(DOCKER_TAG_VERSION)"
@cp -v $(3) $(OUT_DIR)/Dockerfile-$(1)

# Building and pushing in one step is more efficient with buildx, so we duplicate the command
# to build and add --push.
cd $(OUT_DIR) && docker buildx build -f ./Dockerfile-$(1) \
--platform linux/amd64,linux/arm64,linux/arm \
-t $(DOCKER_REGISTRY)/$(1):$(DOCKER_TAG_VERSION) \
--label org.opencontainers.image.source="$(IMAGE_SRC)" \
--label org.opencontainers.image.description="$(1)" \
--label org.opencontainers.image.version="$(REL_VERSION)" \
--label org.opencontainers.image.revision="$(GIT_COMMIT)" \
--push $(2)
--push \
$(2)
endef

# configure-buildx is to initialize qemu and buildx environment.
Expand Down Expand Up @@ -100,15 +117,18 @@ DOCKER_BUILD_TARGETS:=$(foreach IMAGE,$(DOCKER_IMAGES),docker-build-$(IMAGE))
# list of 'outputs' to push all images
DOCKER_PUSH_TARGETS:=$(foreach IMAGE,$(DOCKER_IMAGES),docker-push-$(IMAGE))

# list of 'outputs' to build all multi arch images
DOCKER_BUILD_MULTI_TARGETS:=$(foreach IMAGE,$(DOCKER_IMAGES),docker-multi-arch-build-$(IMAGE))

# list of 'outputs' to push all multi arch images
DOCKER_PUSH_MULTI_TARGETS:=$(foreach IMAGE,$(DOCKER_IMAGES),docker-multi-arch-push-$(IMAGE))

# targets to build test images
.PHONY: docker-test-image-build
docker-test-image-build: docker-build-magpiego docker-build-testrp ## Builds all Docker images.
docker-test-image-build: docker-build-magpiego docker-build-testrp ## Builds all test Docker images.

.PHONY: docker-test-image-push
docker-test-image-push: docker-push-magpiego docker-push-testrp ## Builds all Docker images.
docker-test-image-push: docker-push-magpiego docker-push-testrp ## Pushes all test Docker images.

# targets to build development images
.PHONY: docker-build
Expand All @@ -119,5 +139,8 @@ docker-push: $(DOCKER_PUSH_TARGETS) docker-test-image-push ## Pushes all Docker

# targets to build and push multi arch images. If you run this target in your machine,
# ensure you have qemu and buildx installed by running make configure-buildx.
.PHONY: docker-multi-arch-build
docker-multi-arch-build: $(DOCKER_BUILD_MULTI_TARGETS) ## Builds all non-test docker images for multiple architectures.

.PHONY: docker-multi-arch-push
docker-multi-arch-push: $(DOCKER_PUSH_MULTI_TARGETS) ## Pushes all docker images after building.
docker-multi-arch-push: $(DOCKER_PUSH_MULTI_TARGETS) ## Pushes all non-test docker images for multiple architectures after building.

0 comments on commit be1d6a1

Please sign in to comment.