exec credential provider: wire in cluster info (superset of #91192) by ankeesler · Pull Request #95489 · kubernetes/kubernetes

ankeesler · 2020-10-12T14:11:49Z

This PR should contain a strict superset of what @enj did in: exec credential provider: wire in cluster info #91192.
- I opened this PR since I don't think I can push to @enj's branch in exec credential provider: wire in cluster info #91192.
- I created my new branch from the tip of @enj's branch in exec credential provider: wire in cluster info #91192, so it should have his same commits, except now I am marked as the committer since I rebased master on top of his stuff.
The KEP for this exec credential feature set is here: https://github.com/kubernetes/enhancements/blob/master/keps/sig-auth/541-external-credential-providers/README.md
This PR should contain functionality agreed upon in the KEP updates
- KEP update 1: Update exec credential provider KEP with feedback from cluster info PR enhancements#1927
- KEP update 2: Update exec credential provider KEP after convo with sig-auth enhancements#2023
The original k/e proposal for this feature set is here: External client-go credential providers enhancements#541

Signed-off-by: Andrew Keesler akeesler@vmware.com

/kind feature
/sig auth
/milestone v1.20

client-go credential plugins can now be passed in the current cluster information via the KUBERNETES_EXEC_INFO environment variable.

Need to update https://kubernetes.io/docs/reference/access-authn-authz/authentication/#client-go-credential-plugins

k8s-ci-robot · 2020-10-12T14:11:52Z

@ankeesler: You must be a member of the kubernetes/milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your and have them propose you as an additional delegate for this responsibility.

Details

In response to this:

This PR should contain a strict superset of what @enj did in: exec credential provider: wire in cluster info #91192.

I opened this PR since I don't think I can push to @enj's branch in exec credential provider: wire in cluster info #91192.

I created my new branch from the tip of @enj's branch in exec credential provider: wire in cluster info #91192, so it should have his same commits, except now I am marked as the committer since I rebased master on top of his stuff.

The KEP for this exec credential feature set is here: https://github.com/kubernetes/enhancements/blob/master/keps/sig-auth/541-external-credential-providers/README.md

This PR should contain functionality agreed upon in the KEP updates

KEP update 1: Update exec credential provider KEP with feedback from cluster info PR enhancements#1927

KEP update 2: Update exec credential provider KEP after convo with sig-auth enhancements#2023

The original k/e proposal for this feature set is here: External client-go credential providers enhancements#541

Signed-off-by: Andrew Keesler akeesler@vmware.com

/kind feature
/sig auth
/milestone v1.20
client-go credential plugins can now be passed in the current cluster information via the KUBERNETES_EXEC_INFO environment variable.
Need to update https://kubernetes.io/docs/reference/access-authn-authz/authentication/#client-go-credential-plugins

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2020-10-12T14:11:58Z

Hi @ankeesler. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

api/api-rules/violation_exceptions.list

ankeesler · 2020-10-12T15:00:55Z

fedebongio · 2020-10-15T20:18:57Z

/cc @cheftako @DangerOnTheRanger
/remove-sig api-machinery

k8s-ci-robot · 2020-10-15T20:18:59Z

@fedebongio: GitHub didn't allow me to request PR reviews from the following users: DangerOnTheRanger.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @cheftako @DangerOnTheRanger
/remove-sig api-machinery

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Signed-off-by: Monis Khan <mok@vmware.com>

ankeesler · 2020-10-26T21:40:19Z

/test pull-kubernetes-node-e2e

liggitt · 2020-10-28T14:29:38Z

staging/src/k8s.io/client-go/tools/clientcmd/api/v1/conversion.go

I don't think this will be necessary once conversion-gen=false is added

liggitt · 2020-10-28T14:30:29Z

staging/src/k8s.io/client-go/tools/clientcmd/api/types.go

add // +k8s:conversion-gen=false to explicitly opt out of conversion

bd62001e034

liggitt · 2020-10-28T14:33:23Z

staging/src/k8s.io/client-go/rest/config_test.go

nit: don't put dummy data indicating a password in the config we document as not being intended to contain passwords :-)

38db9c4b6b3

liggitt · 2020-10-28T14:45:02Z

staging/src/k8s.io/client-go/tools/clientcmd/api/types.go

Suggested change

return fmt.Sprintf("api.AuthProviderConfig{Command: %q, Args: %#v, Env: %s, APIVersion: %q, ProvideClusterInfo: %t, Config: %s}", c.Command, args, env, c.APIVersion, c.ProvideClusterInfo, config)

return fmt.Sprintf("api.ExecConfig{Command: %q, Args: %#v, Env: %s, APIVersion: %q, ProvideClusterInfo: %t, Config: %s}", c.Command, args, env, c.APIVersion, c.ProvideClusterInfo, config)

b46f46b3002

liggitt · 2020-10-28T15:12:42Z

staging/src/k8s.io/client-go/pkg/apis/clientauthentication/v1beta1/types_test.go

This test looks good for catching gaps in both directions.

would this make more sense in staging/src/k8s.io/client-go/tools/auth/exec which already has visibility to both the k8s.io/client-go/tools/clientcmd/api and k8s.io/client-go/pkg/apis/clientauthentication API packages? it seems like it would pair well with the test that ensures we can construct rest.Config objects with fidelity from the envvar.

Sure, sounds reasonable to me. I was nervous to move it to .../tools/... because I didn't know if there was a convention around testing API structs as close to their declaration as possible, but I totally agree with you that k8s.io/client-go/tools/auth/exec seems like the natural place for this since the dependency graph works out nicely.

502dd3fcf0d

liggitt · 2020-10-28T15:14:08Z

staging/src/k8s.io/client-go/pkg/apis/clientauthentication/v1beta1/types_test.go

be explicit about what the expected action is here... typically, it will be to fix the exec cluster config type

77da453042f

liggitt · 2020-10-28T15:20:28Z

staging/src/k8s.io/client-go/rest/exec.go

this is inconsistent with the API doc that indicates the cluster field will be nil if ProvideClusterInfo is false. It looks like we only call this in one non-test location; should we put the burden of doing this check on the caller instead and assume if we are called that config.ExecProvider != nil && config.ExecProvider.ProvideClusterInfo == true?

09323e5c557

This will likely change once we figure out where to put this thing...

staging/src/k8s.io/client-go/rest/transport.go

liggitt · 2020-10-28T15:33:39Z

staging/src/k8s.io/client-go/tools/auth/exec/exec.go

going all the way through a mock kubeconfig struct seems pretty roundabout. why are we not translating these fields directly? we have the following methods which should round-trip (modulo loading CA file and resolving the proxy URL):

execClusterToConfig: exec cluster -> rest.Config

ConfigToExecCluster: rest.Config -> exec cluster

I expected both of these to live in similar places and have a test verifying they could round-trip with each other

going all the way through a mock kubeconfig struct seems pretty roundabout. why are we not translating these fields directly?

Yup, totally agree, it seems silly. I started to write a manual conversion from exec cluster to rest.Config, but then I realized I was rewriting a lot of kubeconfig loading logic. To avoid code duplication, and possibly missing out on future rest.Config-creation features/bug fixes, I elected to create the mock kubeconfig. This is certainly a loosely held opinion, though.

I expected both of these to live in similar places

Agreed. This was another "where should these code elements go" moment. If we put ConfigToExecCluster, per your suggestion above[0], in k8s.io/client-go/plugin/pkg/client/auth/exec, then I guess it would follow existing dependency graph edges, now that I think of it. I thought that plugin packages were only supposed to be imported into main packages, and the like, for registration. However, I'm just now realizing that k8s.io/client-go/rest depends on k8s.io/client-go/plugin/pkg/client/auth/exec, and k8s.io/client-go/tools/auth/exec depends on k8s.io/client-go/rest, so therefore k8s.io/client-go/tools/auth/exec already has a dependency on k8s.io/client-go/plugin/pkg/client/auth/exec. As long as you are OK with me importing k8s.io/client-go/plugin/pkg/client/auth/exec into k8s.io/client-go/tools/auth/exec, I'll do that.

[0] #95489 (comment)

I'm just now realizing that k8s.io/client-go/rest depends on k8s.io/client-go/plugin/pkg/client/auth/exec, and k8s.io/client-go/tools/auth/exec depends on k8s.io/client-go/rest, so therefore k8s.io/client-go/tools/auth/exec already has a dependency on k8s.io/client-go/plugin/pkg/client/auth/exec.

That was my thinking as well

I realized I was rewriting a lot of kubeconfig loading logic

Really? I expected a pretty straight copy of the fields… there's no user or context info. It seems like it would be less work and more straightforward than the field by field copy you are doing here

I'll go with the straight copy of the fields. Could certainly have been that I was making up stories in my head...

Oh nooooo...I think if we followed through with this plan, plugin/pkg/client/auth/exec would depend on rest (because of return type below), which already depends on plugin/pkg/client/auth/exec...

func ExecClusterToConfig(cluster *clientauthentication.Cluster) (*rest.Config, error)

OK new bad idea: put both ExecClusterToConfig and ConfigToExecCluster in rest, roundtrip test them there, continue to inject the clientauthentcation.Cluster into exec.GetAuthenticator, and call ExecClusterToConfig from tools/auth/exec. I think that is the only way the dependency graph will line up.

I think you're right. That's not that bad... put them in their own file and there's a certain symmetry to them

gave this a shot here: 930bfd161a2

liggitt · 2020-10-28T15:34:31Z

staging/src/k8s.io/client-go/rest/exec.go

shouldn't this be passing in the config host? also, Proxy can return a nil url with no error

Suggested change

url, err := config.Proxy(nil)

hostURL, err := url.Parse(config.Host)

...

proxyURL, err := config.Proxy(hostURL)

...

if proxyURL != nil {

... = proxyURL.String()

}

The proxyURL nil check makes sense, but I believe config.Proxy() accepts a *http.Request.

Proxy func(*http.Request) (*url.URL, error)

Maybe config.Proxy(&http.Request{URL: hostURL})? This is kinda a tricky conversion, because we are essentially converting from a dynamic proxy URL (rest.Config.Proxy) to a static proxy URL (clientauthentication.Client). I would feel bad passing a half-baked http.Request to config.Proxy.

mm, true. probably http.NewRequest("", config.Host, nil) to get the same normalization we'd get for actual requests.

0750ed9c614

ankeesler · 2020-10-29T02:09:31Z

staging/src/k8s.io/client-go/tools/auth/exec/types_test.go

I'm all ears for suggestions on how we can make sure we don't forget to do this when we go to GA. I know we can add a TODO item somewhere in GitHub, but I would love to be able to add something in code.

you can check if a ExecCredential is registered for v1 into the scheme created in staging/src/k8s.io/client-go/tools/auth/exec/exec.go and fail with a "TODO: add field fuzzing test" if so (can test it with a check for v1beta1 to make sure the check works)

excellent. 56d9b8fda37

ankeesler · 2020-10-29T14:01:01Z

staging/src/k8s.io/client-go/tools/auth/exec/exec_test.go

How would we feel about a test that validates that the rest.Config returned from LoadExecCredential can actually be used to make a REST call? I worry that we will change something in rest.ExecClusterToConfig and forget to validate that LoadExecCredential still returns a REST config that is valid for HTTP calls. Perhaps this is being too wary...

staging/src/k8s.io/client-go/rest/exec.go

liggitt · 2020-10-29T14:34:41Z

staging/src/k8s.io/client-go/rest/exec.go

Suggested change

return nil, fmt.Errorf("failed to get proxy URL: %w", err)

return nil, fmt.Errorf("failed to get proxy URL for execProvider: %w", err)

b4ce993f7ce

liggitt · 2020-10-29T14:35:01Z

staging/src/k8s.io/client-go/rest/exec.go

Suggested change

return nil, fmt.Errorf("failed to create empty http request: %w", err)

return nil, fmt.Errorf("failed to create proxy URL request for execProvider: %w", err)

b4ce993f7ce

liggitt · 2020-10-29T14:39:01Z

staging/src/k8s.io/client-go/rest/transport.go

make this a pointer, don't dereference, and pass a pointer to GetAuthenticator?

Suggested change

var cluster clientauthentication.Cluster

var cluster *clientauthentication.Cluster

I agree this would clean up the code here in transport.go. I was trying to avoid passing a pointer to GetAuthenticator that could be nil or non-nil, so that GetAuthenticator wouldn't need to worry about limping around with a nil pointer. On the other hand, passing this struct by pointer would alleviate some copying of a Cluster up the stack. I will go ahead and update the GetAuthenticator code, and friends, to take a pointer. It doesn't look like we touch that cluster struct very much right now.

bfe3c622358

liggitt · 2020-10-29T14:40:32Z

staging/src/k8s.io/client-go/tools/auth/exec/exec.go

Suggested change

return nil, nil, fmt.Errorf("cannot create config: %w", err)

return nil, nil, fmt.Errorf("cannot create rest.Config: %w", err)

962c4f64655

liggitt · 2020-10-29T14:40:46Z

staging/src/k8s.io/client-go/tools/auth/exec/exec.go

Suggested change

return nil, nil, errors.New("exec credential does not contain cluster information")

return nil, nil, errors.New("ExecCredential does not contain cluster information")

962c4f64655

liggitt · 2020-10-29T14:42:35Z

staging/src/k8s.io/client-go/tools/auth/exec/exec_test.go

why is this needed? modifying the non-test package scheme in a test-only file doesn't seem like a great idea... I really want to exercise the methods that use the scheme the way non-test code will

I used a Pod down below to validate unmarshalling a runtime.Object that wasn't a clientauthentication.ExecCredential. I'll use a different runtime.Object for this so we don't need to modify the scheme in the test.

962c4f64655

liggitt · 2020-10-29T14:43:36Z

staging/src/k8s.io/client-go/tools/auth/exec/types_test.go

this is fine, we could also iterate through AllKnownTypes and fail if we find any clientauthenticationv1beta1.SchemeGroupVersion.Group / ExecCredential group kinds outside the versions we already expect (v1alpha1 and v1beta1)

Makes sense to me. I guess that would protect against stuff like v1beta2 sneaking in.

962c4f64655

staging/src/k8s.io/client-go/tools/auth/exec/exec_test.go

liggitt · 2020-10-29T15:09:59Z

looks like hack/update-bazel.sh is needed as well

liggitt · 2020-10-29T16:45:19Z

this lgtm, go ahead and squash review commits into layers that make sense

- The main idea here is that we want to 1) prevent potentially large CA bundles from being set in an exec plugin's environment and 2) ensure that the exec plugin is getting everything it needs in order to talk to a cluster. - Avoid breaking existing manual declarations of rest.Config instances by moving exec Cluster to kubeconfig internal type. - Use client.authentication.k8s.io/exec to qualify exec cluster extension. - Deep copy the exec Cluster.Config when we copy a rest.Config. Signed-off-by: Andrew Keesler <akeesler@vmware.com>

Exec plugin implementations should be able to call LoadExecCredentialFromEnv() in order to get everything they need to operate (i.e., cluster information (as long as it is passed in) and optionally per-cluster configuration). Signed-off-by: Andrew Keesler <akeesler@vmware.com>

ankeesler · 2020-10-29T17:44:39Z

squashed into 2 major commits. i kept @enj's commits intact since i wanted to leave him as author on those. i tried to pull apart c4299d1 more, but changing struct fields were caused some widespread changes.

ankeesler · 2020-10-29T18:33:08Z

/retest

ankeesler · 2020-10-29T19:50:54Z

Note: i've been using this exec plugin to run end-to-end tests manually: https://gist.github.com/ankeesler/2502d73d164164fc16b1a54bf4607423.

liggitt

/approve

sorry, noticed one doc issue on the types, lgtm after that is fixed

liggitt · 2020-10-29T19:55:01Z

staging/src/k8s.io/client-go/pkg/apis/clientauthentication/types.go

 	//     ...
 	//     extensions:
-	//     - name: exec  # reserved extension name for per cluster exec config
+	//     - name: client.authentication.k8s.io/exec  # reserved extension name for per cluster exec config


nit: also need to change extensions[exec] mention above

good catch, 409f228

liggitt · 2020-10-29T19:55:19Z

staging/src/k8s.io/client-go/pkg/apis/clientauthentication/v1beta1/types.go

 	//     ...
 	//     extensions:
-	//     - name: exec  # reserved extension name for per cluster exec config
+	//     - name: client.authentication.k8s.io/exec  # reserved extension name for per cluster exec config


extensions[exec] mention above

good catch, 409f228

Hopefully we've fixed all of these references now... Signed-off-by: Andrew Keesler <akeesler@vmware.com>

liggitt · 2020-10-29T21:05:42Z

/lgtm
/approve

k8s-ci-robot · 2020-10-29T21:06:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ankeesler, liggitt

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~api/OWNERS~~ [liggitt]
~~staging/src/k8s.io/client-go/OWNERS~~ [liggitt]

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

ankeesler · 2020-10-29T23:40:43Z

/test pull-kubernetes-e2e-kind-ipv6

ankeesler · 2020-10-30T00:47:55Z

/test pull-kubernetes-e2e-gce-100-performance

ankeesler · 2020-10-30T01:06:10Z

/test pull-kubernetes-node-e2e

k8s-ci-robot · 2020-10-30T01:31:03Z

@ankeesler: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-gce-100-performance	`409f228`	link	`/test pull-kubernetes-e2e-gce-100-performance`
pull-kubernetes-e2e-gce-ubuntu-containerd	`409f228`	link	`/test pull-kubernetes-e2e-gce-ubuntu-containerd`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

ankeesler · 2020-10-30T01:32:12Z

/test pull-kubernetes-e2e-gce-ubuntu-containerd

dims · 2022-08-23T12:53:42Z

test ... ignore me!

k8s-ci-robot requested review from a team, deads2k and dims October 12, 2020 14:12

ankeesler commented Oct 12, 2020

View reviewed changes

api/api-rules/violation_exceptions.list Outdated Show resolved Hide resolved

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 12, 2020

ankeesler mentioned this pull request Oct 13, 2020

exec credential provider: updates after sig-release feedback kubernetes/enhancements#2096

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 13, 2020

k8s-ci-robot requested a review from cheftako October 15, 2020 20:18

k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Oct 15, 2020

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Oct 19, 2020

enj added 2 commits October 19, 2020 08:50

exec credential provider: wire in cluster info

f97422c

Signed-off-by: Monis Khan <mok@vmware.com>

Generated conversions

9956697

Signed-off-by: Monis Khan <mok@vmware.com>

liggitt reviewed Oct 28, 2020

View reviewed changes

ankeesler commented Oct 29, 2020

View reviewed changes

staging/src/k8s.io/client-go/rest/exec.go Outdated Show resolved Hide resolved

liggitt reviewed Oct 29, 2020

View reviewed changes

ankeesler added 2 commits October 29, 2020 13:38

ankeesler force-pushed the ankeesler/enj/f/exec_plugin_cluster branch from 124e555 to 875a46b Compare October 29, 2020 17:42

liggitt reviewed Oct 29, 2020

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 29, 2020

exec credential provider: exec -> client.authentication.k8s.io/exec

409f228

Hopefully we've fixed all of these references now... Signed-off-by: Andrew Keesler <akeesler@vmware.com>

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 29, 2020

k8s-ci-robot merged commit 53913a7 into kubernetes:master Oct 30, 2020

github-actions bot mentioned this pull request Nov 3, 2020

Week Ending November 1, 2020 dev-obs/actus#260

Open

dims removed their request for review April 7, 2022 01:42

micahhausler mentioned this pull request May 11, 2022

Default to beta API for eks get-token aws/aws-cli#6940

Merged

	return fmt.Sprintf("api.AuthProviderConfig{Command: %q, Args: %#v, Env: %s, APIVersion: %q, ProvideClusterInfo: %t, Config: %s}", c.Command, args, env, c.APIVersion, c.ProvideClusterInfo, config)
	return fmt.Sprintf("api.ExecConfig{Command: %q, Args: %#v, Env: %s, APIVersion: %q, ProvideClusterInfo: %t, Config: %s}", c.Command, args, env, c.APIVersion, c.ProvideClusterInfo, config)

-		url, err := config.Proxy(nil)
+hostURL, err := url.Parse(config.Host)
+...
+proxyURL, err := config.Proxy(hostURL)
+...
+if proxyURL != nil {
+  ... = proxyURL.String()
+}

	return nil, fmt.Errorf("failed to get proxy URL: %w", err)
	return nil, fmt.Errorf("failed to get proxy URL for execProvider: %w", err)

	return nil, fmt.Errorf("failed to create empty http request: %w", err)
	return nil, fmt.Errorf("failed to create proxy URL request for execProvider: %w", err)

	var cluster clientauthentication.Cluster
	var cluster *clientauthentication.Cluster

	return nil, nil, fmt.Errorf("cannot create config: %w", err)
	return nil, nil, fmt.Errorf("cannot create rest.Config: %w", err)

	return nil, nil, errors.New("exec credential does not contain cluster information")
	return nil, nil, errors.New("ExecCredential does not contain cluster information")

Conversation

ankeesler commented Oct 12, 2020

Uh oh!

k8s-ci-robot commented Oct 12, 2020

Uh oh!

k8s-ci-robot commented Oct 12, 2020

Uh oh!

Uh oh!

ankeesler commented Oct 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fedebongio commented Oct 15, 2020

Uh oh!

k8s-ci-robot commented Oct 15, 2020

Uh oh!

ankeesler commented Oct 26, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ankeesler Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ankeesler Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ankeesler commented Oct 12, 2020 •

edited

Loading

ankeesler Oct 29, 2020 •

edited

Loading

ankeesler Oct 29, 2020 •

edited

Loading