OCPBUGS-30319: bump lib-go to fix SAs acting as OAuth2 clients #108

liouk · 2024-03-14T10:49:35Z

No description provided.

liouk · 2024-03-14T10:49:59Z

/hold proof PR

liouk · 2024-03-18T08:16:18Z

/retest-required

p0lyn0mial · 2024-04-10T09:44:32Z

go.mod

@@ -5,31 +5,31 @@ go 1.21
 require (
 	github.com/MakeNowJust/heredoc v1.0.0
 	github.com/google/btree v1.1.2
-	github.com/google/go-cmp v0.5.9
+	github.com/google/go-cmp v0.6.0
 	github.com/google/gofuzz v1.2.0
 	github.com/google/uuid v1.6.0


kube is on v1.3.0 - https://github.com/kubernetes/kubernetes/blob/release-1.29/go.mod#L47

this dep was changed earlier, I think it is okay since minor version are compatible.

p0lyn0mial · 2024-04-10T09:47:01Z

go.mod

+	k8s.io/client-go v0.29.2
+	k8s.io/code-generator v0.29.2
+	k8s.io/component-base v0.29.2
+	k8s.io/klog/v2 v2.120.1


kube is on v2.110.1 - https://github.com/kubernetes/kubernetes/blob/release-1.29/go.mod#L114
let's match kube.

This must have been reverted back with some other module upgrade 🤦‍♂️ Fixed

p0lyn0mial · 2024-04-10T09:48:23Z

Also verified if the termination events (openshift/kubernetes-apiserver@8ba4d58) are applied https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_oauth-apiserver/108/pull-ci-openshift-oauth-apiserver-master-e2e-aws/1768660242603708416/artifacts/e2e-aws/gather-must-gather/artifacts/event-filter.html

p0lyn0mial · 2024-04-10T09:50:30Z

pkg/cmd/oauth-apiserver/cmd.go

@@ -49,7 +49,6 @@ func NewOAuthAPIServerOptions(out io.Writer) *OAuthAPIServerOptions {
 		TokenValidationOptions: tokenvalidationoptions.NewTokenValidationOptions(),
 		Output:                 out,
 	}
-	o.RecommendedOptions.Etcd.StorageConfig.Paging = true


why the change ?

storagebackend.Config has no field Paging: https://github.com/openshift/kubernetes-apiserver/blob/openshift-apiserver-4.16-kubernetes-1.29.2/pkg/storage/storagebackend/config.go#L58-L92

ok, the param was removed in kubernetes/kubernetes#121390

p0lyn0mial · 2024-04-10T09:50:44Z

pkg/cmd/oauth-apiserver/cmd.go

@@ -133,7 +132,7 @@ func (o *OAuthAPIServerOptions) NewOAuthAPIServerConfig() (*apiserver.Config, er
 	}

 	serverConfig.GenericConfig.OpenAPIConfig = openapiconfig.DefaultOpenAPIConfig()
-	serverConfig.GenericConfig.OpenAPIV3Config = openapiconfig.DefaultOpenAPIConfig()
+	serverConfig.GenericConfig.OpenAPIV3Config = openapiconfig.DefaultOpenAPIV3Config()


why the change ?

DefaultOpenAPIConfig() returns *common.Config which is incompatible with common.OpenAPIV3config (the type of serverConfig.GenericConfig.OpenAPIV3Config).

p0lyn0mial · 2024-04-10T09:55:05Z

go.mod

 	github.com/google/gofuzz v1.2.0
 	github.com/google/uuid v1.6.0
 	github.com/jteeuwen/go-bindata v3.0.8-0.20151023091102-a0ff2567cfb7+incompatible
-	github.com/openshift/api v0.0.0-20231101013329-0d0d46454bb7
-	github.com/openshift/apiserver-library-go v0.0.0-20230621110634-a99412818848
+	github.com/openshift/api v0.0.0-20231218131639-7a5aa77cc72d


why not to pull much newer version ?

it looks like the same applies to the other openshift/* deps.

p0lyn0mial · 2024-04-10T10:03:52Z

go.mod

+	k8s.io/api => k8s.io/api v0.29.2
+	k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.29.2
+	k8s.io/apimachinery => k8s.io/apimachinery v0.29.2
+	k8s.io/apiserver => github.com/liouk/kubernetes-apiserver v0.0.0-20240313155529-ced3d910063d // points to openshift-apiserver-4.16-kubernetes-1.29.2


don't forget to point it to the real repo :)

p0lyn0mial · 2024-04-10T10:04:42Z

go.mod

+	k8s.io/kube-scheduler => k8s.io/kube-scheduler v0.29.2
+	k8s.io/kubectl => k8s.io/kubectl v0.29.2
+	k8s.io/kubelet => k8s.io/kubelet v0.29.2
+	k8s.io/kubernetes => github.com/openshift/kubernetes v1.29.0-rc.1.0.20240306191509-a0beecc776d4


this should point to k/k repo, right ?

/cc @dgrisonnet

liouk · 2024-04-10T14:14:20Z

Updated PR to point to openshift/[email protected].

p0lyn0mial · 2024-04-11T07:28:53Z

pkg/cmd/oauth-apiserver/openapiconfig/openapiconfig.go

+func DefaultOpenAPIV3Config() *openapicommon.OpenAPIV3Config {
+	defNamer := apiserverendpointsopenapi.NewDefinitionNamer(legacyscheme.Scheme, extensionsapiserver.Scheme, aggregatorscheme.Scheme)
+	return &openapicommon.OpenAPIV3Config{
+		Info: &spec.Info{


It looks like spec.Info could be shared between DefaultOpenAPIV3Config and DefaultOpenAPIConfig, could you refactor ?

Right, indeed they're the same type so that's a good idea 👍

p0lyn0mial · 2024-04-11T07:30:23Z

pkg/cmd/oauth-apiserver/openapiconfig/openapiconfig.go

@@ -115,3 +116,101 @@ func DefaultOpenAPIConfig() *openapicommon.Config {
 		SecurityDefinitions: &securityDefinitions,
 	}
 }
+
+func DefaultOpenAPIV3Config() *openapicommon.OpenAPIV3Config {
+	defNamer := apiserverendpointsopenapi.NewDefinitionNamer(legacyscheme.Scheme, extensionsapiserver.Scheme, aggregatorscheme.Scheme)


It looks like we could create a default config using genericapiserver.DefaultOpenAPIV3Config and then overwrite:

spec.Info

IgnorePrefixes

SecuritySchemes

WDYT ?

Good idea, thanks for pointing out genericapiserver.DefaultOpenAPIV3Config!

OK, once this PR merges can you open a new one for using genericapiserver.DefaultOpenAPIConfig() in DefaultOpenAPIConfig ?

p0lyn0mial · 2024-04-11T07:56:01Z

go.mod

+	k8s.io/kube-aggregator v0.29.2
+	k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00
+	k8s.io/kubernetes v1.29.2
+	k8s.io/utils v0.0.0-20240310230437-4693a0247e57


kube is on v0.0.0-20230726121419-3b25d923346b - https://github.com/kubernetes/kubernetes/blob/release-1.29/go.mod#L129C15-L129C49

liouk · 2024-04-11T10:15:29Z

/retitle OCPBUGS-30319: Bump openshift/kubernetes-apiserver to openshift-apiserver-4.16-kubernetes-1.29.2

openshift-bot · 2024-04-11T10:15:42Z

@liouk: This pull request references Jira Issue OCPBUGS-30319, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.16.0) matches configured target version for branch (4.16.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @dpuniaredhat

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

p0lyn0mial · 2024-04-11T11:03:24Z

LGTM

just two additional comments, do you mind addressing #108 (comment) and #108 (comment) ?

p0lyn0mial · 2024-04-11T12:29:31Z

/lgtm
/approve

liouk · 2024-04-11T12:30:51Z

Proof PR updated and can now be used to perform the actual bump.

/retitle OCPBUGS-30319: bump lib-go to fix SAs acting as OAuth2 clients
/hold cancel

p0lyn0mial · 2024-04-11T12:30:51Z

/hold cancel

liouk · 2024-04-12T10:15:26Z

/retest-required

openshift-ci · 2024-04-12T12:50:59Z

@liouk: all tests passed!

Full PR test history. Your PR dashboard.

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.

stlaz · 2024-04-15T08:24:34Z

/approve

openshift-ci · 2024-04-15T08:25:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liouk, p0lyn0mial, stlaz

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~~ [stlaz]

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

openshift-ci-robot · 2024-04-15T08:29:16Z

@liouk: Jira Issue OCPBUGS-30319: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-30319 has been moved to the MODIFIED state.

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-bot · 2024-04-15T13:45:34Z

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-oauth-apiserver-container-v4.16.0-202404150912.p0.g87c15a4.assembly.stream.el9 for distgit ose-oauth-apiserver.
All builds following this will include this PR.

openshift-merge-robot · 2024-04-16T01:38:55Z

Fix included in accepted release 4.16.0-0.nightly-2024-04-15-184947

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 14, 2024

openshift-ci bot requested review from deads2k and mfojtik March 14, 2024 10:49

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 14, 2024

liouk mentioned this pull request Mar 14, 2024

[openshift-apiserver-4.16-kubernetes-1.29.2] Add carry patches from 4.15 openshift/kubernetes-apiserver#56

Merged

liouk force-pushed the prove-kubernetes-apiserver-56 branch from 4f272b5 to db416a9 Compare March 14, 2024 13:47

liouk changed the title ~~WIP: Proof for https://github.com/openshift/kubernetes-apiserver/pull/56~~ Proof for https://github.com/openshift/kubernetes-apiserver/pull/56 Mar 15, 2024

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 15, 2024

liouk force-pushed the prove-kubernetes-apiserver-56 branch 2 times, most recently from c7ea533 to c17f3ef Compare March 15, 2024 15:26

p0lyn0mial reviewed Apr 10, 2024

View reviewed changes

openshift-ci bot requested a review from dgrisonnet April 10, 2024 10:06

liouk force-pushed the prove-kubernetes-apiserver-56 branch from c17f3ef to 389f619 Compare April 10, 2024 13:52

liouk force-pushed the prove-kubernetes-apiserver-56 branch from 389f619 to bcef04e Compare April 10, 2024 14:20

p0lyn0mial reviewed Apr 11, 2024

View reviewed changes

liouk force-pushed the prove-kubernetes-apiserver-56 branch from bcef04e to 016f361 Compare April 11, 2024 09:13

openshift-ci bot changed the title ~~Proof for https://github.com/openshift/kubernetes-apiserver/pull/56~~ OCPBUGS-30319: Bump openshift/kubernetes-apiserver to openshift-apiserver-4.16-kubernetes-1.29.2 Apr 11, 2024

openshift-ci bot requested a review from dpuniaredhat April 11, 2024 10:15

liouk added 4 commits April 11, 2024 14:26

bump(kubernetes,kubernetes-apiserver,api,k8s.io/*)

195e9a8

vendor: update

412159a

pkg: fix compile errors after bump to k8s 1.29

e163a10

run make update

adcee10

liouk force-pushed the prove-kubernetes-apiserver-56 branch from 016f361 to adcee10 Compare April 11, 2024 12:27

openshift-ci bot assigned p0lyn0mial Apr 11, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 11, 2024

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 11, 2024

openshift-ci bot changed the title ~~OCPBUGS-30319: Bump openshift/kubernetes-apiserver to openshift-apiserver-4.16-kubernetes-1.29.2~~ OCPBUGS-30319: bump lib-go to fix SAs acting as OAuth2 clients Apr 11, 2024

openshift-ci-robot mentioned this pull request Apr 12, 2024

OCPBUGS-24987, OCPBUGS-30319: bump lib-go to fix SAs acting as OAuth2 clients openshift/oauth-server#142

Merged

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 15, 2024

openshift-merge-bot bot merged commit 87c15a4 into openshift:master Apr 15, 2024
8 checks passed

liouk mentioned this pull request Apr 23, 2024

NO-ISSUE: openapiconfig: generate base config using DefaultOpenAPIConfig #111

Merged

OCPBUGS-30319: bump lib-go to fix SAs acting as OAuth2 clients #108

OCPBUGS-30319: bump lib-go to fix SAs acting as OAuth2 clients #108

Conversation

liouk commented Mar 14, 2024

liouk commented Mar 14, 2024

liouk commented Mar 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

p0lyn0mial commented Apr 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liouk commented Apr 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liouk commented Apr 11, 2024

openshift-bot commented Apr 11, 2024

p0lyn0mial commented Apr 11, 2024

p0lyn0mial commented Apr 11, 2024

liouk commented Apr 11, 2024

p0lyn0mial commented Apr 11, 2024

liouk commented Apr 12, 2024

openshift-ci bot commented Apr 12, 2024

stlaz commented Apr 15, 2024

openshift-ci bot commented Apr 15, 2024

openshift-ci-robot commented Apr 15, 2024

openshift-bot commented Apr 15, 2024

openshift-merge-robot commented Apr 16, 2024