projectriff / riff

riff is for functions
https://projectriff.io
Apache License 2.0
799 stars 64 forks source link

Hardcoded container registry for knative-serving #1355

Closed rudolfv closed 5 years ago

rudolfv commented 5 years ago

I am currently busy with a POC for using knative serving in our organization and came across the following issue. I deployed the uppercase sample for the java-function-invoker using the following command:

riff function create uppercase --local-path . --handler uppercase --image myartifactory.mydomain.com:5003/riff-test/uppercase --namespace riff-test --invoker java

Then I ran the following command to create a deployer:

riff knative deployer create knative-uppercase --function-ref uppercase --namespace riff-test

The user-container image correctly references our artifactory. However the queue-proxy image points to gcr.io.

image: gcr.io/knative-releases/github.com/knative/serving/cmd/queue@sha256:89fb5a1d2d9c0abd10ce3135c02f9e9ffbf93087a3ece7481615a0f9d9209713

We are behind a corporate firewall and would like to use artifactory for the gcr.io images.

I don't see any argument for riff knative deployer create which would override this. Also manually editing the deployment using kubectl edit deployment does not work - after applying an edit and running kubectl edit deployment again, the old values are still there. Editing the pod does work, but obviously this is not a good short term solution as the deployer pods are deployed and removed all the time.

After editing the pod I could invoke the function using:

curl http://my-istio-ingress-gateway-loadbalancer/ -w '\n' -H 'Host: knative-uppercase.riff-test.example.com' -H 'Content-Type: text/plain' -d 'test'

And it succesfully returned: TEST

I had the same problem with the riff installation using the helm charts. I got around that by manually editing each of the deployments in the riff-system, knative-serving and knative-build namespaces. I had a look at the helm chart (riff-0.4.0) and it's hardcoded there too.

rudolfv commented 5 years ago

It looks like doing a search and replace of "image: gcr.io" in the helm chart fixed the issue for me. However, I would recommend making this a configurable value in the official helm chart.

scothis commented 5 years ago

The Knative chart is a relatively naive wrapping of the Knative release artifact as there is no official chart. The projectriff/charts repo contains scripts that generate the chart by applying ytt transforms to the static release. It should be possible to create a transform that replaces the hard coded images with a templated value.

Pivotal Function Service (PFS) supports image relocation. The upcoming v0.4.0 release uses a CNAB bundle to manage the distribution and relocation of images in addition to adjusting the configuration that is applied to the cluster to use the relocated images. While the actual bundle is a commercial offering, the core components are available in open source.

scothis commented 5 years ago

The queue-proxy sidecar image is defined in the config-deployment config map.

rudolfv commented 5 years ago

Thank you @scothis. I'll close this issue.