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.

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

Skaffold dev fails when user cannot list deployments at cluster scope #2495

Closed
corneliusweig opened this issue Jul 18, 2019 · 8 comments · Fixed by #2651
Closed

Skaffold dev fails when user cannot list deployments at cluster scope #2495

corneliusweig opened this issue Jul 18, 2019 · 8 comments · Fixed by #2651
Assignees
Labels
kind/bug Something isn't working priority/p0 Highest priority. We are actively looking at delivering it.

Comments

@corneliusweig
Copy link
Contributor

The deployment status check from Skaffold tries to list all deployments at cluster scope, when running Skaffold without the --namespace flag. For example:

cd examples/nodejs
kubectl auth can-i list deploy --all-namespaces  # -> no
skaffold dev

Fails with

FATA[0001] exiting dev mode because first deploy failed: could not fetch deployments: could not fetch deployments: deployments.apps is forbidden: User "default-user" cannot list resource "deployments" in API group "apps" at the cluster scope

On the other hand:

kubectl auth can-i list deploy -n default  # -> yes
skaffold dev -n default

works as expected.


The reason for this is that pkg/skaffold/deploy/status_check.go:80 does

client.AppsV1().Deployments(ns).List(metav1.ListOptions{
		LabelSelector: l.K8sManagedByLabelKeyValueString(),
	})

with ns=="" (due to not specifying a namespace), and that means all namespaces.

@balopat balopat added kind/bug Something isn't working priority/p0 Highest priority. We are actively looking at delivering it. labels Jul 18, 2019
@balopat
Copy link
Contributor

balopat commented Jul 18, 2019

We should assume default namespace here if it's not given.

@corneliusweig
Copy link
Contributor Author

corneliusweig commented Jul 18, 2019

There are further cases that should be accounted for. For example:

  • the namespace can be configured per deployment in deployment.metadata
  • and if there's no metadata.namespace, the default namespace is whatever is configured in kubeconfig
  • if there's still no namespace, it defaults to default

EDIT: Maybe that's too many special cases. I just looked at AggregatePodWatcher and it uses runCtx.namespaces as source of truth.

@balopat
Copy link
Contributor

balopat commented Jul 18, 2019

We will turn the status check off by default #2499, that's going to downgrade this to priority/p1.
And then we can have a measured look at this.

I agree that there is a difference between leaving it empty - unconstrained vs setting it to default which would override resource and kubeconfig namespaces...

@corneliusweig
Copy link
Contributor Author

Can somebody re-open this issue please? (I cannot)

There is a very similar bug when retrieving the services for automatic port-forwarding. The error happens in pkg/skaffold/kubernetes/portforward/resource_forwarder.go:104, specifically in

services, err := clientset.CoreV1().Services("").List(metav1.ListOptions{
		LabelSelector: label,
	})

@TomaszKlosinski
Copy link

This breaks my RBAC. We have a namespace for each developer and we share skaffold.yaml within the git repo. Everybody suppose to work in his own namespace, but without access to the cluster-wide deployments. How can make it work just with current context namespace (like it did in previous version of skaffold)?

@nkubala nkubala added the triage/discuss Items for discussion label Aug 14, 2020
@briandealwis briandealwis removed the triage/discuss Items for discussion label Aug 24, 2020
@nkubala nkubala reopened this Aug 24, 2020
@nkubala
Copy link
Contributor

nkubala commented Aug 24, 2020

reopening this until we can verify our fixes are working as intended.

@tejal29
Copy link
Contributor

tejal29 commented Aug 28, 2020

I verified this issue here on latest skaffold.
We had a bug were empty namespaces were added to opts.Namespaces
with this fix, #4608 this does not happen anymore.

  1. Create a user which can only deploy in "test" namespace.
kubectl config use-context employee-context
Switched to context "employee-context".

kubectl auth can-i list deployment --all-namespaces
no

kubectl auth can-i list deployment -n test 
yes
  1. Deploy in test namespace
skaffold dev -d gcr.io/tejal-test -n test
Waiting for deployments to stabilize...
 - test:deployment/leeroy-app: container leeroy-app is waiting to start: gcr.io/tejal-test/leeroy-app:v1.13.0-54-g035c89fa1@sha256:0fdf31c9ed55125f07edee2434e787e8b765f263e8f949437128ddaf632ddf9e can't be pulled
    - test:pod/leeroy-app-6c778c4d85-ltzln: container leeroy-app is waiting to start: gcr.io/tejal-test/leeroy-app:v1.13.0-54-g035c89fa1@sha256:0fdf31c9ed55125f07edee2434e787e8b765f263e8f949437128ddaf632ddf9e can't be pulled
 - test:deployment/leeroy-app failed. Error: container leeroy-app is waiting to start: gcr.io/tejal-test/leeroy-app:v1.13.0-54-g035c89fa1@sha256:0fdf31c9ed55125f07edee2434e787e8b765f263e8f949437128ddaf632ddf9e can't be pulled.
 - test:deployment/leeroy-web: creating container leeroy-web
    - test:pod/leeroy-web-6c95b49f5-dvmcp: creating container leeroy-web
 - test:deployment/leeroy-web failed. Error: creating container leeroy-web.
Cleaning up...
 - deployment.apps "leeroy-web" deleted
 - service "leeroy-app" deleted
 - deployment.apps "leeroy-app" deleted

@erulabs
Copy link

erulabs commented Nov 14, 2023

Heya - just a comment that @tejal29's example does not resolve this issue. The issue is running skaffold dev or skaffold run without specifying a namespace. This still occurs with skaffold v2.9.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working priority/p0 Highest priority. We are actively looking at delivering it.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants