Refactor CI workflow for Azure Docker integration #12
Conversation
…ate Kubernetes manifests for Azure deployment
…and metadata extraction
There was a problem hiding this comment.
Pull request overview
This PR migrates the CI/CD pipeline and Kubernetes deployment infrastructure from Google Cloud Platform (GCP) to Microsoft Azure. The changes remove GCP-specific configurations and introduce Azure-based Docker image management with Azure Container Registry (ACR) and updated Kubernetes manifests for AKS deployment.
Key Changes:
- Updated CI workflow to authenticate with Azure and push images to Azure Container Registry (
gabby.azurecr.io) - Removed GCP-specific Kubernetes configurations (base directory and prod overlay with GCP Artifact Registry, Cloud DNS, IAM)
- Created new Azure overlay with updated Istio configurations, certificate management, and deployment manifests targeting the new domain
dsa.cat-herding.net
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Replaced GCP Workload Identity Federation with Azure login and ACR authentication; added Docker Buildx and metadata actions for improved image tagging |
docs/GITHUB_ACTIONS_NOTES.md |
Updated documentation to reflect Azure service principal setup and ACR usage instead of GCP |
k8s/base/* |
Removed all base Kubernetes manifests (deployment, service, DNS, certificates, auth) as they are now defined in the Azure overlay |
k8s/overlays/prod/* |
Removed entire prod overlay including GCP-specific Flux image automation, IAM policies, and artifact registry configurations |
k8s/overlays/azure/kustomization.yaml |
Created new Azure overlay kustomization referencing Azure-specific resources |
k8s/overlays/azure/app.yaml |
Added complete application definition with deployment, service, service account, and authorization policy |
k8s/overlays/azure/gateway-default.yaml |
Created Istio Gateway configuration for AKS with updated API version (v1) and AKS-specific ingress selector |
k8s/overlays/azure/virtualservice-default.yaml |
Created Istio VirtualService for the new domain with updated API version (v1) |
k8s/overlays/azure/certificate-dsa-cat-herding.yaml |
Added cert-manager Certificate resource for the new domain with Let's Encrypt integration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
docs/GITHUB_ACTIONS_NOTES.md
Outdated
| - Authenticates to Azure using [`azure/login`](https://github.com/Azure/login) with the `AZURE_CREDENTIALS` secret (JSON output from `az ad sp create-for-rbac --sdk-auth`). | ||
| - Logs in to Azure Container Registry (ACR) `gabby`. | ||
| - Uses `docker/setup-buildx-action` + `docker/metadata-action` for reproducible builds with cache reuse. | ||
| - Builds and pushes the Docker image to `gabby.azurecr.io/python-dsa` with tags: `latest` (default branch), branch/tag refs, and the commit SHA. | ||
|
|
||
| ### Azure Service Principal Setup | ||
|
|
||
| - Create (or reuse) a service principal with AcrPush permissions on the `gabby` registry and store the JSON output as the `AZURE_CREDENTIALS` secret: | ||
|
|
||
| ```bash | ||
| gcloud iam service-accounts add-iam-policy-binding \ | ||
| ${{ GCP_SERVICE_ACCOUNT }} \ | ||
| --role="roles/iam.workloadIdentityUser" \ | ||
| --member="principalSet://iam.googleapis.com/projects/${{ GCP_PROJECT_NUMBER }}/locations/global/workloadIdentityPools/${{ GCP_WIF_POOL }}/attribute.repository/ianlintner/python_dsa" | ||
| az ad sp create-for-rbac \ | ||
| --name python-dsa-github-ci \ | ||
| --role AcrPush \ | ||
| --scopes /subscriptions/79307c77-54c3-4738-be2a-dc96da7464d9/resourceGroups/nekoc/providers/Microsoft.ContainerRegistry/registries/gabby \ | ||
| --sdk-auth |
There was a problem hiding this comment.
The --sdk-auth flag for az ad sp create-for-rbac has been deprecated by Azure. Consider using the newer federated identity approach with OIDC instead, or if you must use this method, note that it's deprecated and may be removed in future Azure CLI versions.
For federated identity (recommended):
az ad app create --display-name python-dsa-github-ci
az ad app federated-credential create --id <app-id> --parameters credential.jsonWhere credential.json configures GitHub OIDC trust.
| name: python-dsa-policy | ||
| spec: |
There was a problem hiding this comment.
The AuthorizationPolicy is missing a namespace field in the metadata. Without an explicit namespace, it will be created in the default namespace, which may not properly apply to your workload if it's deployed in a different namespace.
Additionally, this policy lacks a selector field in the spec to specify which workloads it should apply to. Without a selector, this policy applies to all workloads in the namespace, which may be overly permissive.
Consider adding:
metadata:
name: python-dsa-policy
namespace: default # or your target namespace
spec:
selector:
matchLabels:
app: python-dsa
action: ALLOW
# ... rest of spec| name: python-dsa-policy | |
| spec: | |
| name: python-dsa-policy | |
| namespace: default | |
| spec: | |
| selector: | |
| matchLabels: | |
| app: python-dsa |
k8s/overlays/azure/app.yaml
Outdated
| metadata: | ||
| labels: | ||
| app: python-dsa | ||
| istio-injection: enabled |
There was a problem hiding this comment.
The pod template has both the label istio-injection: enabled (line 23) and the annotation sidecar.istio.io/inject: "true" (line 25), which are redundant.
The label istio-injection: enabled is used for namespace-level injection, while the annotation is used for pod-level control. Since you're explicitly setting the annotation on the pod, the label on the pod template is unnecessary and could cause confusion.
Consider removing the label from the pod template:
labels:
app: python-dsa
# Remove: istio-injection: enabledIf you want namespace-level injection, apply the label to the namespace instead.
| istio-injection: enabled |
.github/workflows/ci.yml
Outdated
| needs: tests | ||
| permissions: | ||
| contents: 'read' | ||
| id-token: 'write' |
There was a problem hiding this comment.
The id-token: 'write' permission is specified but not used in this job. This permission is typically needed for OIDC/Workload Identity Federation with cloud providers, but the current implementation uses the deprecated AZURE_CREDENTIALS secret approach which doesn't require OIDC tokens.
If you plan to migrate to Azure federated identity with OIDC (which is recommended over --sdk-auth), this permission would be needed. Otherwise, consider removing it to follow the principle of least privilege:
permissions:
contents: 'read'
# Remove: id-token: 'write' # Not needed for secret-based auth| id-token: 'write' |
…te documentation for clarity on triggers and Azure integration
Update the CI workflow to utilize Azure for Docker image management and enhance Kubernetes manifests for deployment on Azure. This change improves the build and push process for Docker images while ensuring proper integration with Azure services.