search

Azure-Samples / kubernetes-offer-samples

Samples for creating an Azure Marketplace Kubernetes offer
MIT License
17 stars 27 forks source link

CPA verification fails for images defined in globals.azure.images section even if syntactically correct #39

Open nuvious opened 3 months ago

nuvious commented 3 months ago

This issue is for a: (mark with an x)

- [ X] bug report -> please search issues before submitting
- [ ] feature request
- [ ] documentation issue or request
- [ ] regression (a behavior that used to work and stopped in a new release)

Minimal steps to reproduce

NOTE: I have forked and committed the below changes to a branch here and a diff of the changes can be seen here

cd samples/k8s-offer-azure-vote
mkdir charts
cd charts
helm create test-subchart-redis

Edit image section in samples/k8s-offer-azure-vote/azure-vote/charts/test-subchart-redis/values.yaml to read:

image:
  repository: redis
  pullPolicy: IfNotPresent
  # Overrides the image tag whose default is the chart appVersion.
  tag: "alpine"

Edit liveness/readiness probes in the samples/k8s-offer-azure-vote/azure-vote/charts/test-subchart-redis/values.yaml to read:

livenessProbe:
  tcpSocket:
    port: 6379
  initialDelaySeconds: 15
  periodSeconds: 10
readinessProbe:
  tcpSocket:
    port: 6379
  initialDelaySeconds: 15
  periodSeconds: 10

Edit globals.azure.images section to read:

global:
  azure:
    images:
      frontend:
        tag: latest
        image: azure-vote-front
        registry: azurek8ssamples.azurecr.io/marketplaceimages
      backend:
        tag: alpine
        image: redis
        registry: docker.io/library
      test-subchart-redis:
        tag: alpine
        image: redis
        registry: docker.io/library

NOTE: I have also tried putting the globals.azure.images definition in the subchart itself but that does not resolve the issue.

Additionally, there are a few places in the generated helm chart where you need/may want to change nginx to redis. Again, if you clone the branch in my fork, I have done this already.

Test helm chart with helm install azure-vote-with-subchart samples/k8s-offer-azure-vote/azure-vote. Expected output with kubectl get all:

drcheese@drcheese-workstation:~/kubernetes-offer-samples$ helm install azure-vote-with-subchart samples/k8s-offer-azure-vote/azure-vote
NAME: azure-vote-with-subchart
# other output from helm chart
kubectl get service -l name=azure-vote-front -w
drcheese@drcheese-workstation:~/kubernetes-offer-samples$ kubectl get app
error: the server doesn't have a resource type "app"
drcheese@drcheese-workstation:~/kubernetes-offer-samples$ kubectl get all
NAME                                                               READY   STATUS    RESTARTS   AGE
pod/azure-vote-with-subchart-test-subchart-redis-fd686547c-xsfh9   1/1     Running   0          37s
pod/vote-back-azure-vote-with-subchart-558558cd8d-5hgpm            1/1     Running   0          37s
pod/vote-front-azure-vote-with-subchart-74788c556f-2b88z           1/1     Running   0          37s

NAME                                                   TYPE           CLUSTER-IP    EXTERNAL-IP       PORT(S)        AGE
service/azure-vote-front                               LoadBalancer   10.0.30.199   172.183.224.209   80:30733/TCP   37s
service/azure-vote-with-subchart-test-subchart-redis   ClusterIP      10.0.77.65    <none>            80/TCP         37s
service/vote-back-azure-vote-with-subchart             ClusterIP      10.0.34.29    <none>            6379/TCP       37s

NAME                                                           READY   UP-TO-DATE   AVAILABLE   AGE
deployment.apps/azure-vote-with-subchart-test-subchart-redis   1/1     1            1           37s
deployment.apps/vote-back-azure-vote-with-subchart             1/1     1            1           37s
deployment.apps/vote-front-azure-vote-with-subchart            1/1     1            1           37s

NAME                                                                     DESIRED   CURRENT   READY   AGE
replicaset.apps/azure-vote-with-subchart-test-subchart-redis-fd686547c   1         1         1       37s
replicaset.apps/vote-back-azure-vote-with-subchart-558558cd8d            1         1         1       37s
replicaset.apps/vote-front-azure-vote-with-subchart-74788c556f           1         1         1       37s
drcheese@drcheese-workstation:~/kubernetes-offer-samples$

Run cpa verify with the samples/k8s-offer-azure-vote directory as your PWD:

# cd samples/k8s-offer-azure-vote
docker run -it --rm \
    -v /var/run/docker.sock:/var/run/docker.sock \
    -v $PWD:/data \
    mcr.microsoft.com/container-package-app:latest \
    /bin/sh -c "cd /data; cpa verify"

Any log messages given by the failure

CPA Version:  1.3.12
By using the Azure Kubernetes CNAB Packaging Application, you agree to the License contained in the application. You can view the License at ~/LICENSE. 
We collect telemetry data, if you would like to opt out of data collection please use the --telemetryOptOut flag.
Correlation Id: aeb883e5-95d6-46f6-b7d0-e4355ab3e4d9
Manifest file validated, 0 total failure(s)

Manifest verification successful.
Helm chart validated, 1 total failure(s)

  Container image was invalid: The container image 'docker.io/library/redis:alpine' generated from helm was not listed in the values.yaml file under global.azure.images.
2024/05/21 17:59:53 
Correlation Id: aeb883e5-95d6-46f6-b7d0-e4355ab3e4d9, CPABuild: 1.3.12
2024/05/21 17:59:53 Container image was invalid: The container image 'docker.io/library/redis:alpine' generated from helm was not listed in the values.yaml file under global.azure.images.
2024/05/21 17:59:53 
For more info refer to the documentation here: https://aka.ms/K8sOfferPrepareAssets#update-the-helm-chart

Expected/desired behavior

Chart is verified. The container referenced is a repeat of previously used syntax so it should be syntactically correct.

OS and Version?

drcheese@drcheese-workstation:~/kubernetes-offer-samples/samples/k8s-offer-azure-vote$ cat /etc/os-release 
PRETTY_NAME="Ubuntu 22.04.4 LTS"
NAME="Ubuntu"
VERSION_ID="22.04"
VERSION="22.04.4 LTS (Jammy Jellyfish)"
VERSION_CODENAME=jammy
ID=ubuntu
ID_LIKE=debian
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
UBUNTU_CODENAME=jammy

Versions

CPA Version: 1.3.12 Image used when running docker: mcr.microsoft.com/container-package-app:latest (per reference below)

Mention any other details that might be useful

References used: https://learn.microsoft.com/en-us/partner-center/marketplace/azure-container-technical-assets-kubernetes?tabs=windows%2Clinux2 NOTE: I would have raised this issue with the CPA tool if it had a github but it doesn't seem to. Additionally, the https://mcr.microsoft.com does not seem to have any reference information for this container definition which could point to a project page where this issue could be raise. Happy to re-publish the issue somewhere appropriate if this isn't the right project. Just came here because the above link references this project specifically.

thomasyip-msft commented 3 months ago

image

You need to update this reference to use global.azure...

Here is the fix: image

image

image

nuvious commented 3 months ago

image

You need to update this reference to use global.azure...

like image: "{{ .Values.global.azure.images.test-subchart-redis.registry }}/{{ .Values.global.azure.images.test-subchart-redis.image }}:{{ .Values.global.azure.images.test-subchart-redis.tag }}"

So if I'm understanding correctly; I have to modify dependency subcharts from different vendors; (specifically their templates) to get them to work? That means every time I need to update a subchart to a new version I have to modify it? In the other project I'm working on that has this issue, we're pulling in these subcharts via helm dependencies and using the helm dependencies build command. This pulls tgz archives of the charts down that the CPA tool then instructs needs to be uncompressed. After decompression the cpa tool has no issues reading the charts but comes across the errors described in this issue.

I did this workflow in this issue because it was the simplest way to demonstrate the error, but pulling in subcharts from 3rd party vendor dependencies and then requiring users to modify how the 3rd party vendor charts reference images does not seem reasonable or easily scalable from a project management perspective. If I need to bump the version of my redis, minio, etc containers, I just want to modify the section in the helm dependencies, run the dependency build and go. I can deal with having to uncompress these as it can be easy to automate. Going into each subchart template and editing how the 3rd party vendor references images does not seem reasonable though.

Is there another way this could be handled or can the CPA tool be enhanced to remove this requirement. I shouldn't have to edit how subcharts reference images and requiring that adds a lot of overhead to updating dependencies and/or further introduces Azure specific syntax/schema requirements into our helm charts which ideally should be platform agnostic.

nuvious commented 3 months ago

P.S. If the purpose of the global.azure.images section in the main chart's values.yaml file is simply to register all the container images needed to build the cnab bundle, this could be achieved without that section by just rendering the helm template and using appropriate regex to find the images referenced:

helm template any-random-name . | sed -nr "s/^\s+image:\s[\"'](.+)[\"']$/\1/p" | sort | uniq

The cpa tool does have the helm command available in its structure:

drcheese@drcheese-workstation:~/kubernetes-offer-samples$ cd samples/k8s-offer-azure-vote/azure-vote/
drcheese@drcheese-workstation:~/kubernetes-offer-samples/samples/k8s-offer-azure-vote/azure-vote$ docker run -it --rm -v /var/run/docker.sock:/v
ar/run/docker.sock -v $PWD:/data --entrypoint "/bin/bash" mcr.microsoft.com/container-package-app:latest
root [ / ]# cd data
root [ /data ]# helm template placeholder-name . | sed -nr "s/^\s+image:\s[\"'](.+)[\"']$/\1/p" | sort | uniq
docker.io/library/redis:alpine
redis:alpine
root [ /data ]#
thomasyip-msft commented 3 months ago

Thanks for your detail reply. Currently this is required as all images that is referenced by each Kubernetes application will be uploaded and referenced through Microsoft owned registry, which will get continuous vulnerability scanning. This include all sub-charts that is being referenced as that is part of one's application requirement.

Your suggestion is very helpful, and we will prioritize this to improve in the future release to reduce the number of changes required by our publisher going forward.