fix: prepend https:// in service-account-issuer flag #4262

chewong · 2021-02-12T00:04:02Z

Signed-off-by: Ernest Wong chuwon@microsoft.com

Reason for Change:

Fixes test failure in https://testgrid.k8s.io/sig-release-master-informing#aks-engine-azure-windows-master-containerd, specifically the following test case:

Kubernetes e2e suite.[sig-auth] ServiceAccounts ServiceAccountIssuerDiscovery should support OIDC discovery of service account issuer [Conformance]

Got the following error when executing that test case:

2021/02/11 20:32:47 Get "kubernetes.default.svc/.well-known/openid-configuration": unsupported protocol scheme ""

I manually updated some of the API models used for testing and looks like we need explicitly set the protocol by prepending https:// in the service-account-issuer flag to fix the test failure.

Issue Fixed:

Credit Where Due:

Does this change contain code from or inspired by another project?

No
Yes

If "Yes," did you notify that project's maintainers and provide attribution?

No
Yes

Requirements:

uses conventional commit messages
includes documentation
adds unit tests
tested upgrade from previous version

Notes:

jackfrancis · 2021-02-12T00:14:25Z

are we missing functional aks-engine E2E tests?

codecov · 2021-02-12T00:19:44Z

Codecov Report

Merging #4262 (884cd24) into master (6a8a5af) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4262      +/-   ##
==========================================
- Coverage   72.07%   72.06%   -0.01%     
==========================================
  Files         141      141              
  Lines       21676    21634      -42     
==========================================
- Hits        15622    15590      -32     
+ Misses       5105     5093      -12     
- Partials      949      951       +2

Impacted Files	Coverage Δ
pkg/api/defaults-apiserver.go	`100.00% <100.00%> (ø)`
pkg/helpers/azure_skus.go	`91.66% <0.00%> (-8.34%)`	⬇️
cmd/get_versions.go	`89.47% <0.00%> (-5.27%)`	⬇️
pkg/engine/params_k8s.go	`79.57% <0.00%> (-0.56%)`	⬇️
pkg/engine/armvariables.go	`85.74% <0.00%> (-0.40%)`	⬇️
pkg/engine/template_generator.go	`68.20% <0.00%> (-0.15%)`	⬇️
pkg/api/defaults-custom-cloud-profile.go	`85.12% <0.00%> (-0.13%)`	⬇️
pkg/api/addons.go	`98.03% <0.00%> (-0.08%)`	⬇️
pkg/engine/artifacts.go	`98.90% <0.00%> (-0.05%)`	⬇️
pkg/api/convertertoapi.go	`94.01% <0.00%> (-0.02%)`	⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a8a5af...884cd24. Read the comment docs.

aramase

lgtm

What is the plan to switch http://aka.ms/aks-engine/aks-engine-k8s-e2e.tar.gz to use the nightly build? If it's not anytime soon, then I'll also need to open a PR to update the dual-stack api models.

chewong · 2021-02-12T19:38:31Z

@aramase @jackfrancis any suggestion on what kind of test we can run here?

chewong · 2021-02-12T21:09:17Z

/hold
doing some additional upstream valdiation

chewong · 2021-03-02T21:20:39Z

The conformance test case above was fixed for containerd + windows cluster but it's still failing for dockershim + windows cluster with the following error:

[BeforeEach] [sig-auth] ServiceAccounts
  /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e/framework/framework.go:185
ï¿½[1mSTEPï¿½[0m: Creating a kubernetes client
Mar  2 13:04:08.548: INFO: >>> kubeConfig: /root/tmp206508484/kubeconfig/kubeconfig.eastus2.json
ï¿½[1mSTEPï¿½[0m: Building a namespace api object, basename svcaccounts
ï¿½[1mSTEPï¿½[0m: Binding the e2e-test-privileged-psp PodSecurityPolicy to the default service account in svcaccounts-255
ï¿½[1mSTEPï¿½[0m: Waiting for a default service account to be provisioned in namespace
[It] ServiceAccountIssuerDiscovery should support OIDC discovery of service account issuer [Conformance]
  /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e/framework/framework.go:640
Mar  2 13:04:08.960: INFO: created pod
Mar  2 13:04:08.960: INFO: Waiting up to 5m0s for pod "oidc-discovery-validator" in namespace "svcaccounts-255" to be "Succeeded or Failed"
Mar  2 13:04:08.995: INFO: Pod "oidc-discovery-validator": Phase="Pending", Reason="", readiness=false. Elapsed: 34.811084ms
Mar  2 13:04:11.055: INFO: Pod "oidc-discovery-validator": Phase="Pending", Reason="", readiness=false. Elapsed: 2.094364037s
Mar  2 13:04:13.090: INFO: Pod "oidc-discovery-validator": Phase="Pending", Reason="", readiness=false. Elapsed: 4.129168121s
Mar  2 13:04:15.124: INFO: Pod "oidc-discovery-validator": Phase="Pending", Reason="", readiness=false. Elapsed: 6.163303348s
Mar  2 13:04:17.158: INFO: Pod "oidc-discovery-validator": Phase="Pending", Reason="", readiness=false. Elapsed: 8.197157296s
Mar  2 13:04:19.192: INFO: Pod "oidc-discovery-validator": Phase="Failed", Reason="", readiness=false. Elapsed: 10.231445528s
Mar  2 13:04:49.194: INFO: polling logs
Mar  2 13:04:49.239: INFO: Pod logs: 
2021/03/02 13:04:13 OK: Got token
2021/03/02 13:04:14 OK: got issuer https://kubernetes.default.svc.cluster.local
2021/03/02 13:04:14 Full, not-validated claims: 
openidmetadata.claims{Claims:jwt.Claims{Issuer:"https://kubernetes.default.svc.cluster.local", Subject:"system:serviceaccount:svcaccounts-255:default", Audience:jwt.Audience{"oidc-discovery-test"}, Expiry:1614690849, NotBefore:1614690249, IssuedAt:1614690249, ID:""}, Kubernetes:openidmetadata.kubeClaims{Namespace:"svcaccounts-255", ServiceAccount:openidmetadata.kubeName{Name:"default", UID:"f1f72bc5-b3f2-43ca-95ab-5e42016f49bf"}}}
2021/03/02 13:04:14 Get "https://kubernetes.default.svc.cluster.local/.well-known/openid-configuration": dial tcp: lookup kubernetes.default.svc.cluster.local: no such host

still not sure what causes the dial tcp: lookup kubernetes.default.svc.cluster.local: no such host error. Tried debugging with a dnsutil container:

apiVersion: v1
kind: Pod
metadata:
  name: dnsutils
  namespace: default
spec:
  containers:
  - name: dnsutils
    image: k8s.gcr.io/e2e-test-images/jessie-dnsutils:1.4
    command:
      - sleep
      - "3600"
    imagePullPolicy: IfNotPresent
  restartPolicy: Always

and then run

kubectl exec dnsutils -- nslookup kubernetes.default.svc.cluster.local

Server:		10.0.0.10
Address:	10.0.0.10#53

Name:	kubernetes.default.svc.cluster.local
Address: 10.0.0.1

Everything looks good.

chewong · 2021-03-02T21:21:03Z

/cc @jsturtevant

jsturtevant · 2021-03-08T21:22:45Z

We identified the issue for Windows and opened a PR to fix the docker scenario in Windows: kubernetes/kubernetes#99860

chewong · 2021-03-08T21:35:17Z

@jackfrancis @aramase added an e2e test case. PTAL.

chewong · 2021-03-08T21:36:14Z

test/e2e/kubernetes/workloads/pod-projected-svc-token.yaml

+  name: nginx
+spec:
+  containers:
+  - image: k8s.gcr.io/e2e-test-images/nginx:1.14-1


This is a nginx test image we use upstream, which supports both Linux and Windows.

Signed-off-by: Ernest Wong <chuwon@microsoft.com>

aramase

/lgtm

chewong · 2021-03-10T23:39:15Z

@jackfrancis can we move forward with this PR. Would love to get this PR merged so we can have green test signal on testgrid. Thanks in advance.

jackfrancis

/lgtm

acs-bot · 2021-03-10T23:47:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aramase, chewong, jackfrancis

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [jackfrancis]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

chewong requested a review from jackfrancis February 12, 2021 00:04

aramase reviewed Feb 12, 2021

View reviewed changes

mboersma added the do-not-merge/hold label Feb 16, 2021

chewong removed the do-not-merge/hold label Mar 8, 2021

chewong commented Mar 8, 2021

View reviewed changes

fix: prepend https:// in service-account-issuer flag

4a39a95

Signed-off-by: Ernest Wong <chuwon@microsoft.com>

chewong mentioned this pull request Mar 9, 2021

ServiceAccounts ServiceAccountIssuerDiscovery should support OIDC discovery of service account issuer [Conformance] kubernetes/kubernetes#99470

Closed

Add service-account-issuser override

884cd24

Signed-off-by: Ernest Wong <chuwon@microsoft.com>

aramase reviewed Mar 10, 2021

View reviewed changes

jackfrancis approved these changes Mar 10, 2021

View reviewed changes

jackfrancis merged commit f81f85f into Azure:master Mar 10, 2021

chewong deleted the service-account-issuer branch March 11, 2021 00:02

chewong mentioned this pull request Mar 19, 2021

api-model: remove --service-account-issuer kubernetes-sigs/cloud-provider-azure#566

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: prepend https:// in service-account-issuer flag #4262

fix: prepend https:// in service-account-issuer flag #4262

chewong commented Feb 12, 2021 •

edited

Loading

jackfrancis commented Feb 12, 2021

codecov bot commented Feb 12, 2021 •

edited

Loading

aramase left a comment

chewong commented Feb 12, 2021

chewong commented Feb 12, 2021

chewong commented Mar 2, 2021

chewong commented Mar 2, 2021

jsturtevant commented Mar 8, 2021

chewong commented Mar 8, 2021

chewong Mar 8, 2021

aramase left a comment

chewong commented Mar 10, 2021

jackfrancis left a comment

acs-bot commented Mar 10, 2021

fix: prepend https:// in service-account-issuer flag #4262

fix: prepend https:// in service-account-issuer flag #4262

Conversation

chewong commented Feb 12, 2021 • edited Loading

jackfrancis commented Feb 12, 2021

codecov bot commented Feb 12, 2021 • edited Loading

Codecov Report

aramase left a comment

Choose a reason for hiding this comment

chewong commented Feb 12, 2021

chewong commented Feb 12, 2021

chewong commented Mar 2, 2021

chewong commented Mar 2, 2021

jsturtevant commented Mar 8, 2021

chewong commented Mar 8, 2021

chewong Mar 8, 2021

Choose a reason for hiding this comment

aramase left a comment

Choose a reason for hiding this comment

chewong commented Mar 10, 2021

jackfrancis left a comment

Choose a reason for hiding this comment

acs-bot commented Mar 10, 2021

chewong commented Feb 12, 2021 •

edited

Loading

codecov bot commented Feb 12, 2021 •

edited

Loading