Ambassador: Allow ambassador Helm chart to use CRDS when namespaced

[volatilemolotov]

Describe the bug
I have a ambassador instance deployed as a namespaced resource and get the following

2019-05-29 08:34:51 kubewatch [32 TMainThread] 0.70.1 DEBUG: looking up ID for namespace god-default
2019-05-29 08:34:51 kubewatch [32 TMainThread] 0.70.1 DEBUG: CRD type definition unreadable for authservices.getambassador.io: Forbidden
2019-05-29 08:34:51 kubewatch [32 TMainThread] 0.70.1 DEBUG: CRD type definition unreadable for mappings.getambassador.io: Forbidden
2019-05-29 08:34:51 kubewatch [32 TMainThread] 0.70.1 DEBUG: CRD type definition unreadable for modules.getambassador.io: Forbidden
2019-05-29 08:34:51 kubewatch [32 TMainThread] 0.70.1 DEBUG: CRD type definition unreadable for ratelimitservices.getambassador.io: Forbidden
2019-05-29 08:34:51 kubewatch [32 TMainThread] 0.70.1 DEBUG: CRD type definition unreadable for tcpmappings.getambassador.io: Forbidden
2019-05-29 08:34:51 kubewatch [32 TMainThread] 0.70.1 DEBUG: CRD type definition unreadable for tlscontexts.getambassador.io: Forbidden
2019-05-29 08:34:52 kubewatch [32 TMainThread] 0.70.1 DEBUG: CRD type definition unreadable for tracingservices.getambassador.io: Forbidden
2019-05-29 08:34:52 kubewatch [32 TMainThread] 0.70.1 DEBUG: CRDs are not available. To enable CRD support, configure the Ambassador CRD type definitions and RBAC, then restart the Ambassador pod.

To Reproduce
Steps to reproduce the behavior:

1. Deploy crds
2. Deploy ambassador using namespaced: true
3. ambassador rolls back to annotation only mode

Expected behavior
Ambassador should be able to read CRDS depoyed within its namespace without using ClusterRole RBAC definitions

Versions (please complete the following information):

• Ambassador: 0.70.1
• Kubernetes environment GKE

[kflynn]

You're deploying with the Helm chart?

[bourquep]

Facing the same issue. Installed Ambassador 0.70.1 via Helm, like this:

helm install stable/ambassador --namespace ambassador --name ambassador

(using helm v2.11.0)

helm list

NAME            REVISION    UPDATED                     STATUS      CHART               APP VERSION NAMESPACE        
ambassador      1           Sat May 25 16:34:36 2019    DEPLOYED    ambassador-2.6.1    0.70.1      ambassador       

kubectl get crd

NAME                                 CREATED AT
authservices.getambassador.io        2019-05-25T20:34:36Z
mappings.getambassador.io            2019-05-25T20:34:36Z
modules.getambassador.io             2019-05-25T20:34:36Z
ratelimitservices.getambassador.io   2019-05-25T20:34:36Z
tcpmappings.getambassador.io         2019-05-25T20:34:36Z
tlscontexts.getambassador.io         2019-05-25T20:34:36Z
tracingservices.getambassador.io     2019-05-25T20:34:36Z

And the Ambassador admin UI shows this error:

[volatilemolotov]

Also helm, version 0.70.1

[richarddli]

cc @Flydiverny I think this is an issue with the Helm chart.

[Flydiverny]

Please see if https://github.com/helm/charts/pull/14388 would solve your problems :)

[volatilemolotov]

@Flydiverny so basically it won't request to list namespaces?, how can i test this?

[volatilemolotov]

Took your branch and tried to install it , get the following error:
clusterroles.rbac.authorization.k8s.io "ambassador-test-default-view-crds" is forbidden:

Used the following values in values.yaml:

rbac:
  # Specifies whether RBAC resources should be created
  create: true
  namespaced: true

scope:
  # tells Ambassador to only use resources in the namespace or namespace set by namespace.name
  singleNamespace: true
crds:
  create: false
  keep: true

[Flydiverny]

@volatilemolotov Thanks, will see if I can make an update later today :)

Wrote another comment in the PR https://github.com/helm/charts/pull/14388#issuecomment-498043124

I think it will be hard to allow CRDs when RBAC is namespaced as ambassador 0.70.1 requires the ability to see if the definitions exists, which are cluster wide resources and there by requires a cluster role to see, AFAIK.

[volatilemolotov]

Then namespaced ambassador should have ClusterRole but should only read stuff within its namespace (solved in ambassador code)

[Flydiverny]

@kflynn
Would it be possible for Ambassador to see if CRDs exists in its namespace without having to be able to check if the CRD definitions exist? Ie would it be possible to have a fallback when the watch fails (like in 0.70.0) instead of the added definition check in 0.70.1?

[richarddli]

cc @containscafeine who implemented the check

[volatilemolotov]

Why even check if a defintion exists, can it work by just checking if any of the resources from the getambassador.io api group are present in the namespace (or in the cluster if ambassador is cluster scoped) and if not you can just throw a mesasge " no resorces found, check if definitions are present" or something similar

[kflynn]

So here's the issue: suppose that you have a new Ambassador user who sets up a bunch of stuff with CRDs but doesn't quite get it right. If they forget to create the CRD types at all, that's not so bad: Kubernetes won't let them kubectl apply the CRDs and they'll see that something is wrong.

If they get RBAC wrong, though, that's much more annoying. How do we report the error??

Right now, the only channels we have for error reporting are the diagnostics UI or kubectl logs, both of which are asynchronous... and, worse, if Ambassador doesn't see your CRDs at all, it can easily end up in a state where you can't reach the diagnostics UI at all. That's why we originally just had Ambassador crash if it couldn't see the CRDs: it's the only way to be even mostly sure that the user will look at the logs. Unfortunately it was just too painful as people upgraded to 0.70.0 without reading the notes... :wink:

So, yeah. I don't like requiring so many permissions, I'm just not sure of a good way to let users know what's wrong when things are broken. Opinions welcome!!

[bourquep]

I don't have a problem with how you are reporting the error. My issue is more about the fact that a fresh Helm chart install doesn't work out-of-the-box...

[kflynn]

@bourquep _That_ is definitely a bug, yes.

[Flydiverny]

@bourquep I'm unsure what doesn't work with the chart out of the box? If the default settings fail to install or start up please create an issue :)

[bourquep]

@Flydiverny I have documented what is happening here in this current issue.

Basically, helm install leads to Ambassador complaining that the CRDs aren't available, but they are.

[Flydiverny]

@bourquep Please try chart version 2.6.2 or newer. The issue in chart version 2.6.1 was fixed in https://github.com/helm/charts/pull/14155 and is somewhat unrelated to this issue (namespaced vs non namespaced)

[volatilemolotov]

Best way IMHO is to fallback to annotations if no custom resources are found and log it in the stdout. If someone cannot reach its mapping or ambasador itself first thing would be to check the logs. Pod should not crash because you still can use annotations which can be considered as a ready state

[volatilemolotov]

Or it can be solved by using a simple clusterrole with just list watch and get for CRDS like this if ambasasdor is set as namespaced:

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: ambassador-crd
rules:
- apiGroups:
  - apiextensions.k8s.io
  resources:
  - customresourcedefinitions
  verbs:
  - get
  - list
  - watch
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: ambassador-crd
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: ambassador-crd
subjects:
- kind: ServiceAccount
  name: ambassador-namespace
  namespace: namespace

Think this is the least privilege needed to enable crd support (tested on 0.71.0 and it works).
Unfortunately still does not help with Helm given that the tiller needs to be cluster scoped while im using miltiple tiller instances that are namespace scoped

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.