-
Notifications
You must be signed in to change notification settings - Fork 29
chore: support deploying to Openshift and add gitlab pipeline #121
Conversation
6319279
to
1137a7f
Compare
a82049f
to
98d7f8f
Compare
@@ -117,6 +121,11 @@ deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in | |||
cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG} | |||
$(KUSTOMIZE) build config/default | kubectl apply -f - | |||
|
|||
openshift-deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config. | |||
VAL="${RH_RBAC_IMAGE}" yq e '.spec.template.spec.containers[0].image = strenv(VAL)' -i config/default/manager_auth_proxy_patch.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is yq
supposed to be jq
? If so are we assuming it's installed by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think yq is to yaml as jq is to json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, yq is to yaml what jq is to json, exactly. I call out its requirement in the description of this PR: The openshift-deploy Make target requires yq to be installed (see https://github.com/mikefarah/yq).
- I'll update the docs when the time comes but we have a separate task for that - also, it is expected that most people will still install the operator through the catalog and this Make target is probably going to be for development only, so I don't think it's unreasonable to add another dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I had the page open before you updated the description, so I didn't see it. Sounds good.
openshift.Dockerfile
Outdated
|
||
FROM registry.access.redhat.com/ubi8/ubi-minimal:latest | ||
WORKDIR / | ||
COPY --from=builder --chown=65532:65532 /workspace/manager . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to remove chown
? 65532
is not in UBI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks Luca
@@ -117,6 +121,11 @@ deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in | |||
cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG} | |||
$(KUSTOMIZE) build config/default | kubectl apply -f - | |||
|
|||
openshift-deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config. | |||
VAL="${RH_RBAC_IMAGE}" yq e '.spec.template.spec.containers[0].image = strenv(VAL)' -i config/default/manager_auth_proxy_patch.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think yq is to yaml as jq is to json
c08e6f4
to
0b8e75b
Compare
Proposed changes
Add changes to support deploying to Openshift, including new Makefile targets and a RedHat specific Dockerfile. The
openshift-deploy
Make target requires yq to be installed (see https://github.com/mikefarah/yq). This PR also addresses CVE-2020-29652 and adds a gitlab pipeline for testing.Checklist
Before creating a PR, run through this checklist and mark each as complete.