Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update Dashboard addon to version 1.8.0 and align /ui redirect with it #53046

Merged
merged 1 commit into from Dec 1, 2017
+115 −28
Merged

Update Dashboard addon to version 1.8.0 and align /ui redirect with it #53046

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion cluster/addons/dashboard/README.md
View file
Open in desktop
@@ -1,5 +1,4 @@
# Kubernetes Dashboard
==============

Kubernetes Dashboard is a general purpose, web-based UI for Kubernetes clusters.
It allows users to manage applications running in the cluster, troubleshoot them,
Expand Down
9 changes: 9 additions & 0 deletions cluster/addons/dashboard/dashboard-configmap.yaml
View file
Open in desktop
@@ -0,0 +1,9 @@
apiVersion: v1
kind: ConfigMap
metadata:
labels:
k8s-app: kubernetes-dashboard
# Allows editing resource and makes sure it is created first.
addonmanager.kubernetes.io/mode: EnsureExists
name: kubernetes-dashboard-settings
namespace: kube-system
34 changes: 29 additions & 5 deletions cluster/addons/dashboard/dashboard-controller.yaml
View file
Open in desktop
@@ -1,4 +1,13 @@
apiVersion: extensions/v1beta1
apiVersion: v1
kind: ServiceAccount
metadata:
labels:
k8s-app: kubernetes-dashboard
addonmanager.kubernetes.io/mode: Reconcile
name: kubernetes-dashboard
namespace: kube-system
---
apiVersion: apps/v1beta2
kind: Deployment
metadata:
name: kubernetes-dashboard
Expand All @@ -20,23 +29,38 @@ spec:
spec:
containers:
- name: kubernetes-dashboard
image: gcr.io/google_containers/kubernetes-dashboard-amd64:v1.6.3
image: gcr.io/google_containers/kubernetes-dashboard-amd64:v1.8.0
resources:
# keep request = limit to keep this container in guaranteed class
limits:
cpu: 100m
memory: 300Mi
requests:
cpu: 100m
memory: 100Mi
ports:
- containerPort: 9090
- containerPort: 8443
protocol: TCP
args:
- --auto-generate-certificates
volumeMounts:
- name: kubernetes-dashboard-certs
mountPath: /certs
- name: tmp-volume
mountPath: /tmp
livenessProbe:
httpGet:
scheme: HTTPS
path: /
port: 9090
port: 8443
initialDelaySeconds: 30
timeoutSeconds: 30
volumes:
- name: kubernetes-dashboard-certs
secret:
secretName: kubernetes-dashboard-certs
- name: tmp-volume
emptyDir: {}
serviceAccountName: kubernetes-dashboard
tolerations:
- key: "CriticalAddonsOnly"
operator: "Exists"
45 changes: 45 additions & 0 deletions cluster/addons/dashboard/dashboard-rbac.yaml
View file
Open in desktop
@@ -0,0 +1,45 @@
kind: Role
apiVersion: rbac.authorization.k8s.io/v1
metadata:
labels:
k8s-app: kubernetes-dashboard
addonmanager.kubernetes.io/mode: Reconcile
name: kubernetes-dashboard-minimal
namespace: kube-system
rules:
# Allow Dashboard to create 'kubernetes-dashboard-key-holder' secret.
Copy link
Member

@liggitt liggitt Nov 28, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason not to create the kubernetes-dashboard-key-holder secret via manifest with EnsureExists? wouldn't that remove the need for the dashboard to create the secret (it could just get/update the specifically named secrets)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case it should work as addon manager will manage it. Normally, without this permission if secret would not exist Dashboard would not be able to start.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@floreks can the kubernetes-dashboard-key-holder be created empty via manifest, and this create permission removed? will the dashboard populate the empty secret?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately not. If empty secret is found then it will be deleted and during next resync period it is recreated from local copy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's unfortunate. this is ok for now, but I'd like to see that switch to an update and this permission get removed in a future update. can you open an issue for that and link it here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What in case dashboard won't be able to recreate it and someone deletes required secret? It will crash Dashboard for good until secret is restored. This would only work in case secret is managed by addon manager.

Copy link
Member

@liggitt liggitt Nov 29, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from the dashboard's perspective, if it's missing, creating seems ok. if it's empty, I'd expect population via update.

from this manifest's perspective, I'd rather not give secret create permission in the kube-system namespace

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I have created issue for that kubernetes/dashboard#2629

- apiGroups: [""]
resources: ["secrets"]
verbs: ["create"]
# Allow Dashboard to get, update and delete Dashboard exclusive secrets.
- apiGroups: [""]
resources: ["secrets"]
resourceNames: ["kubernetes-dashboard-key-holder", "kubernetes-dashboard-certs"]
verbs: ["get", "update", "delete"]
# Allow Dashboard to get and update 'kubernetes-dashboard-settings' config map.
- apiGroups: [""]
resources: ["configmaps"]
resourceNames: ["kubernetes-dashboard-settings"]
verbs: ["get", "update"]
# Allow Dashboard to get metrics from heapster.
- apiGroups: [""]
resources: ["services"]
resourceNames: ["heapster"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need https:heapster: and http:heapster: as well? cc @DirectXMan12

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want to support connecting to a secured Heapster, then yes, you explicitly need https:heapster:. IIRC, http:heapster: shouldn't be needed b/c that'd be the same as just heapster, unless I'm misremembering.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opened kubernetes/dashboard#2622 to switch to service subresource and add policy for protocol-specific heapster calls

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this blocks this PR, though

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current version of Dashboard does not support connecting to secured Heapster. This can stay as it is.

verbs: ["proxy"]
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: kubernetes-dashboard-minimal
namespace: kube-system
labels:
k8s-app: kubernetes-dashboard
addonmanager.kubernetes.io/mode: Reconcile
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: kubernetes-dashboard-minimal
subjects:
- kind: ServiceAccount
name: kubernetes-dashboard
namespace: kube-system
10 changes: 10 additions & 0 deletions cluster/addons/dashboard/dashboard-secret.yaml
View file
Open in desktop
@@ -0,0 +1,10 @@
apiVersion: v1
kind: Secret
metadata:
labels:
k8s-app: kubernetes-dashboard
# Allows editing resource and makes sure it is created first.
addonmanager.kubernetes.io/mode: EnsureExists
name: kubernetes-dashboard-certs
namespace: kube-system
type: Opaque
4 changes: 2 additions & 2 deletions cluster/addons/dashboard/dashboard-service.yaml
View file
Open in desktop
Expand Up @@ -11,5 +11,5 @@ spec:
selector:
k8s-app: kubernetes-dashboard
ports:
- port: 80
targetPort: 9090
- port: 443
targetPort: 8443
18 changes: 6 additions & 12 deletions cluster/centos/deployAddons.sh
View file
Open in desktop
Expand Up @@ -45,19 +45,13 @@ function deploy_dns {
}

function deploy_dashboard {
if ${KUBECTL} get rc -l k8s-app=kubernetes-dashboard --namespace=kube-system | grep kubernetes-dashboard-v &> /dev/null; then
echo "Kubernetes Dashboard replicationController already exists"
else
echo "Creating Kubernetes Dashboard replicationController"
${KUBECTL} create -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-controller.yaml
fi
echo "Deploying Kubernetes Dashboard"

if ${KUBECTL} get service/kubernetes-dashboard --namespace=kube-system &> /dev/null; then
echo "Kubernetes Dashboard service already exists"
else
echo "Creating Kubernetes Dashboard service"
${KUBECTL} create -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-service.yaml
fi
${KUBECTL} apply -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-secret.yaml
${KUBECTL} apply -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-configmap.yaml
${KUBECTL} apply -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-rbac.yaml
${KUBECTL} apply -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-controller.yaml
${KUBECTL} apply -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-service.yaml

echo
}
Expand Down
7 changes: 5 additions & 2 deletions hack/local-up-cluster.sh
View file
Open in desktop
Expand Up @@ -792,8 +792,11 @@ function start_kubedashboard {
if [[ "${ENABLE_CLUSTER_DASHBOARD}" = true ]]; then
echo "Creating kubernetes-dashboard"
# use kubectl to create the dashboard
${KUBECTL} --kubeconfig="${CERT_DIR}/admin.kubeconfig" create -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-controller.yaml
${KUBECTL} --kubeconfig="${CERT_DIR}/admin.kubeconfig" create -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-service.yaml
${KUBECTL} --kubeconfig="${CERT_DIR}/admin.kubeconfig" apply -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-secret.yaml
${KUBECTL} --kubeconfig="${CERT_DIR}/admin.kubeconfig" apply -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-configmap.yaml
${KUBECTL} --kubeconfig="${CERT_DIR}/admin.kubeconfig" apply -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-rbac.yaml
${KUBECTL} --kubeconfig="${CERT_DIR}/admin.kubeconfig" apply -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-controller.yaml
${KUBECTL} --kubeconfig="${CERT_DIR}/admin.kubeconfig" apply -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-service.yaml
echo "kubernetes-dashboard deployment and service successfully deployed."
fi
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/routes/ui.go
View file
Open in desktop
Expand Up @@ -22,7 +22,7 @@ import (
"k8s.io/apiserver/pkg/server/mux"
)

const dashboardPath = "/api/v1/namespaces/kube-system/services/kubernetes-dashboard/proxy"
const dashboardPath = "/api/v1/namespaces/kube-system/services/https:kubernetes-dashboard:/proxy/"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's no guarantee that the dashboard was deployed using the addon manifest... see discussion in #53766 and #56074 (comment) about why this should be removed, not perpetuated.

Hardcoding a different value here does not make it more wrong, but also does not make it more correct. I still intend to include deprecation notes in 1.9 for the /ui redirect, sweep and remove references to it, and remove it in a future version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liggitt I can understand it. Can we update it just before to make it work or should it be kept broken?

there's no guarantee that the dashboard was deployed using the addon manifest...

I agree. We try to use HTTPS setup everywhere as recommended one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to removing this, but I don't think it should block this PR. Can we release note that it will be going away in 1.10 and make this change for 1.9?

Copy link
Member

@liggitt liggitt Nov 27, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we release note that it will be going away in 1.10 and make this change for 1.9?

That seems ok. Something like this?

Updated Dashboard add-on to version 1.8.0.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


// UIRedirect redirects /ui to the kube-ui proxy path.
type UIRedirect struct{}
Expand Down
1 change: 1 addition & 0 deletions test/e2e/ui/BUILD
View file
Open in desktop
Expand Up @@ -15,6 +15,7 @@ go_library(
"//vendor/github.com/onsi/gomega:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/net:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
],
)
Expand Down
12 changes: 7 additions & 5 deletions test/e2e/ui/dashboard.go
View file
Open in desktop
Expand Up @@ -23,6 +23,7 @@ import (

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
utilnet "k8s.io/apimachinery/pkg/util/net"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/kubernetes/test/e2e/framework"
testutils "k8s.io/kubernetes/test/utils"
Expand All @@ -36,6 +37,7 @@ var _ = SIGDescribe("Kubernetes Dashboard", func() {
uiServiceName = "kubernetes-dashboard"
uiAppName = uiServiceName
uiNamespace = metav1.NamespaceSystem
uiRedirect = "/ui"

serverStartTimeout = 1 * time.Minute
)
Expand Down Expand Up @@ -63,20 +65,20 @@ var _ = SIGDescribe("Kubernetes Dashboard", func() {
ctx, cancel := context.WithTimeout(context.Background(), framework.SingleCallTimeout)
defer cancel()

// Query against the proxy URL for the kube-ui service.
// Query against the proxy URL for the kubernetes-dashboard service.
err := proxyRequest.Namespace(uiNamespace).
Context(ctx).
Name(uiServiceName).
Name(utilnet.JoinSchemeNamePort("https", uiServiceName, "")).
Timeout(framework.SingleCallTimeout).
Do().
StatusCode(&status).
Error()
if err != nil {
if ctx.Err() != nil {
framework.Failf("Request to kube-ui failed: %v", err)
framework.Failf("Request to kubernetes-dashboard failed: %v", err)
return true, err
}
framework.Logf("Request to kube-ui failed: %v", err)
framework.Logf("Request to kubernetes-dashboard failed: %v", err)
} else if status != http.StatusOK {
framework.Logf("Unexpected status from kubernetes-dashboard: %v", status)
}
Expand All @@ -88,7 +90,7 @@ var _ = SIGDescribe("Kubernetes Dashboard", func() {
By("Checking that the ApiServer /ui endpoint redirects to a valid server.")
var status int
err = f.ClientSet.CoreV1().RESTClient().Get().
AbsPath("/ui").
AbsPath(uiRedirect).
Timeout(framework.SingleCallTimeout).
Do().
StatusCode(&status).
Expand Down