feat: add support for deployment cancellation and error surface when admission webhook blocks pod skaffold is waiting on #8624

aaron-prindle · 2023-04-03T05:39:11Z

Manually tested against GKE cluster w/ binauthz enabled following the guide here:
https://cloud.google.com/architecture/binary-auth-with-cloud-build-and-gke

Before this change:

aprindle@aprindle-ssd ~/bad-image  [bad-image]$ skaffold apply manifests.yaml 
Starting deploy...
 - deployment.apps/bad-image configured
Waiting for deployments to stabilize...
 - deployment/bad-image: waiting for rollout to finish: 0 out of 1 new replicas have been updated...
 # ...
 # hangs here, waits until statusCheckDeadlineSeconds

After this change:

aprindle@aprindle-ssd ~/bad-image  [bad-image]$ skaffold apply manifests.yaml 
Starting deploy...
 - deployment.apps/bad-image created
Waiting for deployments to stabilize...
 - deployment/bad-image: Failed to create Pod for Deployment bad-image: Error creating: admission webhook "imagepolicywebhook.image-policy.k8s.io" denied the request: Image us-central1-docker.pkg.dev/aprindle-test-cluster/applications/bad-image:0b78553-dirty@sha256:2fca1fa0b2831ff007d1416e690ae8f882aad5a6c490d801ccdc659ed8b96c22 denied by Binary Authorization cluster admission rule for us-central1.prod-cluster. Image us-central1-docker.pkg.dev/aprindle-test-cluster/applications/bad-image:0b78553-dirty@sha256:2fca1fa0b2831ff007d1416e690ae8f882aad5a6c490d801ccdc659ed8b96c22 denied by attestor projects/aprindle-test-cluster/attestors/vulnz-attestor: No attestations found that were valid and signed by a key trusted by the attestor. Image us-central1-docker.pkg.dev/aprindle-test-cluster/applications/bad-image:0b78553-dirty@sha256:2fca1fa0b2831ff007d1416e690ae8f882aad5a6c490d801ccdc659ed8b96c22 denied by attestor projects/aprindle-test-cluster/attestors/qa-attestor: No attestations found that were valid and signed by a key trusted by the attestor
 - deployment/bad-image failed. Error: Failed to create Pod for Deployment bad-image: Error creating: admission webhook "imagepolicywebhook.image-policy.k8s.io" denied the request: Image us-central1-docker.pkg.dev/aprindle-test-cluster/applications/bad-image:0b78553-dirty@sha256:2fca1fa0b2831ff007d1416e690ae8f882aad5a6c490d801ccdc659ed8b96c22 denied by Binary Authorization cluster admission rule for us-central1.prod-cluster. Image us-central1-docker.pkg.dev/aprindle-test-cluster/applications/bad-image:0b78553-dirty@sha256:2fca1fa0b2831ff007d1416e690ae8f882aad5a6c490d801ccdc659ed8b96c22 denied by attestor projects/aprindle-test-cluster/attestors/vulnz-attestor: No attestations found that were valid and signed by a key trusted by the attestor. Image us-central1-docker.pkg.dev/aprindle-test-cluster/applications/bad-image:0b78553-dirty@sha256:2fca1fa0b2831ff007d1416e690ae8f882aad5a6c490d801ccdc659ed8b96c22 denied by attestor projects/aprindle-test-cluster/attestors/qa-attestor: No attestations found that were valid and signed by a key trusted by the attestor.
1/1 deployment(s) failed

codecov · 2023-04-04T01:17:58Z

Codecov Report

Merging #8624 (eb8e9bc) into main (290280e) will decrease coverage by 6.28%.
The diff coverage is 54.47%.

❗ Current head eb8e9bc differs from pull request most recent head ad9e67a. Consider uploading reports for the commit ad9e67a to get more accurate results

@@            Coverage Diff             @@
##             main    #8624      +/-   ##
==========================================
- Coverage   70.48%   64.21%   -6.28%     
==========================================
  Files         515      613      +98     
  Lines       23150    30725    +7575     
==========================================
+ Hits        16317    19729    +3412     
- Misses       5776     9516    +3740     
- Partials     1057     1480     +423

Impacted Files	Coverage Δ
cmd/skaffold/app/cmd/completion.go	`13.04% <0.00%> (-1.25%)`	⬇️
cmd/skaffold/app/cmd/config/list.go	`65.21% <ø> (ø)`
cmd/skaffold/app/cmd/config/set.go	`88.72% <ø> (ø)`
cmd/skaffold/app/cmd/config/util.go	`54.28% <ø> (ø)`
cmd/skaffold/app/cmd/credits.go	`100.00% <ø> (ø)`
cmd/skaffold/app/cmd/credits/export.go	`0.00% <0.00%> (ø)`
cmd/skaffold/app/cmd/deploy.go	`40.90% <0.00%> (-12.94%)`	⬇️
cmd/skaffold/app/cmd/generate_pipeline.go	`60.00% <ø> (ø)`
cmd/skaffold/app/cmd/inspect_modules.go	`65.00% <ø> (ø)`
cmd/skaffold/app/cmd/inspect_profiles.go	`66.66% <ø> (ø)`
... and 39 more

... and 393 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

renzodavid9 · 2023-04-05T18:19:55Z

integration/testdata/helm-multi-config/skaffold/artifacts.json

It looks like we don't need this file? The integration test is using the examples/kustomize. Same will be for the other files in the integration/testdata/helm- folders.

Thanks, missed this. Updated now

renzodavid9 · 2023-04-05T18:21:55Z

integration/binauthz_test.go

+// TestBinAuthZWithDeploy targets the integration-tests-binauthz cluster on k8s-skaffold which has '--binauthz-evaluation-mode=PROJECT_SINGLETON_POLICY_ENFORCE' set
+func TestBinAuthZWithDeploy(t *testing.T) {
+	MarkIntegrationTest(t, NeedsGcp)
+	if os.Getenv("GKE_CLUSTER_NAME") != binauthzClusterName {


I think we need to do a change in Kokoro to make this work, right?

Yes you're correct - currently this won't run in kokoro (but won't affect the test negatively). Plan is to make the kokoro change to add this cluster and possible modify the test harness as well to only run this test in that cluster

…admission webhook blocks pod skaffold is waiting on

…admission webhook blocks pod skaffold is waiting on (#8624) (cherry picked from commit 3f33f60)

pull-request-size bot added the size/M label Apr 3, 2023

aaron-prindle force-pushed the fix-8610 branch 6 times, most recently from b98e114 to d0a4569 Compare April 4, 2023 00:49

pull-request-size bot added size/L and removed size/M labels Apr 4, 2023

aaron-prindle force-pushed the fix-8610 branch 3 times, most recently from 24fc1f0 to eb8e9bc Compare April 4, 2023 17:54

renzodavid9 reviewed Apr 5, 2023

View reviewed changes

feat: add support for deployment cancellation and error surface when …

ad9e67a

…admission webhook blocks pod skaffold is waiting on

aaron-prindle force-pushed the fix-8610 branch from eb8e9bc to ad9e67a Compare April 5, 2023 19:58

pull-request-size bot added size/M and removed size/L labels Apr 5, 2023

renzodavid9 approved these changes Apr 5, 2023

View reviewed changes

aaron-prindle merged commit 3f33f60 into GoogleContainerTools:main Apr 6, 2023
12 checks passed

aaron-prindle added a commit that referenced this pull request May 15, 2023

feat: add support for deployment cancellation and error surface when …

f839499

…admission webhook blocks pod skaffold is waiting on (#8624) (cherry picked from commit 3f33f60)

ericzzzzzzz mentioned this pull request Aug 11, 2023

fix: status check lists all events #9015

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for deployment cancellation and error surface when admission webhook blocks pod skaffold is waiting on #8624

feat: add support for deployment cancellation and error surface when admission webhook blocks pod skaffold is waiting on #8624

aaron-prindle commented Apr 3, 2023 •

edited

codecov bot commented Apr 4, 2023 •

edited

renzodavid9 Apr 5, 2023

aaron-prindle Apr 5, 2023

renzodavid9 Apr 5, 2023

aaron-prindle Apr 5, 2023

feat: add support for deployment cancellation and error surface when admission webhook blocks pod skaffold is waiting on #8624

feat: add support for deployment cancellation and error surface when admission webhook blocks pod skaffold is waiting on #8624

Conversation

aaron-prindle commented Apr 3, 2023 • edited

codecov bot commented Apr 4, 2023 • edited

Codecov Report

renzodavid9 Apr 5, 2023

Choose a reason for hiding this comment

aaron-prindle Apr 5, 2023

Choose a reason for hiding this comment

renzodavid9 Apr 5, 2023

Choose a reason for hiding this comment

aaron-prindle Apr 5, 2023

Choose a reason for hiding this comment

aaron-prindle commented Apr 3, 2023 •

edited

codecov bot commented Apr 4, 2023 •

edited