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

DO-NOT-MERGE: Prove library-go#1902 #747

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
github.com/ghodss/yaml v1.0.0
github.com/google/go-cmp v0.6.0
github.com/openshift/api v0.0.0-20241001152557-e415140e5d5f
github.com/openshift/api v0.0.0-20241101202457-04eb3fd119d2
github.com/openshift/build-machinery-go v0.0.0-20241031155326-6ae126a9cb72
github.com/openshift/client-go v0.0.0-20241001162912-da6d55e4611f
github.com/openshift/library-go v0.0.0-20241120135057-fc703a7407c9
Expand Down Expand Up @@ -121,3 +121,5 @@ require (
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
sigs.k8s.io/yaml v1.4.0 // indirect
)

replace github.com/openshift/library-go => github.com/liouk/library-go v0.0.0-20241210104019-f07c37b790cf
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/ledongthuc/pdf v0.0.0-20220302134840-0c2507a12d80/go.mod h1:imJHygn/1yfhB7XSJJKlFZKl/J+dCPAknuiaGOshXAs=
github.com/liouk/library-go v0.0.0-20241210104019-f07c37b790cf h1:44q9/wsLnrUDAMCTysjU8C0m1hgVmXAe/7Xc07s1dxw=
github.com/liouk/library-go v0.0.0-20241210104019-f07c37b790cf/go.mod h1:l/3SegTa9x+ry2J213bh7+DBofXOOvdrqU4JC9ktJa0=
github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0=
github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
Expand All @@ -148,14 +150,12 @@ github.com/onsi/ginkgo/v2 v2.19.0 h1:9Cnnf7UHo57Hy3k6/m5k3dRfGTMXGvxhHFvkDTCTpvA
github.com/onsi/ginkgo/v2 v2.19.0/go.mod h1:rlwLi9PilAFJ8jCg9UE1QP6VBpd6/xj3SRC0d6TU0To=
github.com/onsi/gomega v1.33.1 h1:dsYjIxxSR755MDmKVsaFQTE22ChNBcuuTWgkUDSubOk=
github.com/onsi/gomega v1.33.1/go.mod h1:U4R44UsT+9eLIaYRB2a5qajjtQYn0hauxvRm16AVYg0=
github.com/openshift/api v0.0.0-20241001152557-e415140e5d5f h1:ya1OmyZm3LIIxI3U9VE9Nyx3ehCHgBwxyFUPflYPWls=
github.com/openshift/api v0.0.0-20241001152557-e415140e5d5f/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo=
github.com/openshift/api v0.0.0-20241101202457-04eb3fd119d2 h1:CguNy+2KzhJ3a3i7e4Bgm/ByfQpSSSPYmF9NLZskoUs=
github.com/openshift/api v0.0.0-20241101202457-04eb3fd119d2/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo=
github.com/openshift/build-machinery-go v0.0.0-20241031155326-6ae126a9cb72 h1:kMM+Ea3YFrcoYS76RhhBA7uELy97JM0gwqnyoy7fxco=
github.com/openshift/build-machinery-go v0.0.0-20241031155326-6ae126a9cb72/go.mod h1:8jcm8UPtg2mCAsxfqKil1xrmRMI3a+XU2TZ9fF8A7TE=
github.com/openshift/client-go v0.0.0-20241001162912-da6d55e4611f h1:FRc0bVNWprihWS0GqQWzb3dY4dkCwpOP3mDw5NwSoR4=
github.com/openshift/client-go v0.0.0-20241001162912-da6d55e4611f/go.mod h1:KiZi2mJRH1TOJ3FtBDYS6YvUL30s/iIXaGSUrSa36mo=
github.com/openshift/library-go v0.0.0-20241120135057-fc703a7407c9 h1:bwIqO3LDkumwfDKTMRzixNHKUqU7yaKTTAKwENi6JOY=
github.com/openshift/library-go v0.0.0-20241120135057-fc703a7407c9/go.mod h1:9B1MYPoLtP9tqjWxcbUNVpwxy68zOH/3EIP6c31dAM0=
github.com/openshift/multi-operator-manager v0.0.0-20241119235446-3c965870ef94 h1:9Z5HQo1KSQogIpQ2tDzrCk4+sDjL/xF+YXBrDP4R36k=
github.com/openshift/multi-operator-manager v0.0.0-20241119235446-3c965870ef94/go.mod h1:Fn/rmcwj4bCuS11UT5TZvzONt7qTjzcd9BCSQkIwQOI=
github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde/go.mod h1:nZgzbfBr3hhjoZnS66nKrHmduYNpc34ny7RK4z5/HM0=
Expand Down
42 changes: 33 additions & 9 deletions pkg/controllers/deployment/deployment_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/client-go/informers"
coreinformers "k8s.io/client-go/informers/core/v1"
Expand Down Expand Up @@ -59,6 +60,7 @@ type oauthServerDeploymentSyncer struct {
podsLister corev1listers.PodLister
proxyLister configv1listers.ProxyLister
routeLister routev1listers.RouteLister
authLister configv1listers.AuthenticationLister

bootstrapUserDataGetter bootstrap.BootstrapUserDataGetter
bootstrapUserChangeRollOut bool
Expand All @@ -73,6 +75,7 @@ func NewOAuthServerWorkloadController(
openshiftClusterConfigClient configv1client.ClusterOperatorInterface,
configInformers configinformer.SharedInformerFactory,
routeInformersForTargetNamespace routeinformers.SharedInformerFactory,
authLister configv1listers.AuthenticationLister,
bootstrapUserDataGetter bootstrap.BootstrapUserDataGetter,
eventsRecorder events.Recorder,
versionRecorder status.VersionGetter,
Expand All @@ -94,6 +97,7 @@ func NewOAuthServerWorkloadController(
proxyLister: configInformers.Config().V1().Proxies().Lister(),
routeLister: routeInformersForTargetNamespace.Route().V1().Routes().Lister(),

authLister: authLister,
bootstrapUserDataGetter: bootstrapUserDataGetter,
}

Expand Down Expand Up @@ -147,17 +151,17 @@ func (c *oauthServerDeploymentSyncer) PreconditionFulfilled(_ context.Context) (
return true, nil
}

func (c *oauthServerDeploymentSyncer) Sync(ctx context.Context, syncContext factory.SyncContext) (*appsv1.Deployment, bool, []error) {
func (c *oauthServerDeploymentSyncer) Sync(ctx context.Context, syncContext factory.SyncContext) (*appsv1.Deployment, bool, bool, []error) {
errs := []error{}

operatorSpec, operatorStatus, _, err := c.operatorClient.GetOperatorState()
if err != nil {
return nil, false, append(errs, err)
return nil, false, false, append(errs, err)
}

proxyConfig, err := c.getProxyConfig()
if err != nil {
return nil, false, append(errs, err)
return nil, false, false, append(errs, err)
}

// resourceVersions serves to store versions of config resources so that we
Expand All @@ -174,7 +178,7 @@ func (c *oauthServerDeploymentSyncer) Sync(ctx context.Context, syncContext fact

configResourceVersions, err := c.getConfigResourceVersions()
if err != nil {
return nil, false, append(errs, err)
return nil, false, false, append(errs, err)
}

resourceVersions = append(resourceVersions, configResourceVersions...)
Expand All @@ -192,7 +196,17 @@ func (c *oauthServerDeploymentSyncer) Sync(ctx context.Context, syncContext fact
// deployment, have RV of all resources
expectedDeployment, err := getOAuthServerDeployment(operatorSpec, proxyConfig, c.bootstrapUserChangeRollOut, resourceVersions...)
if err != nil {
return nil, false, append(errs, err)
return nil, false, false, append(errs, err)
}

if oidcEnabled, err := c.oidcEnabled(); err != nil {
return nil, false, false, append(errs, err)
} else if oidcEnabled {
err := c.deployments.Deployments(expectedDeployment.Namespace).Delete(ctx, expectedDeployment.Name, metav1.DeleteOptions{})
if err != nil {
return nil, false, false, append(errs, err)
}
return nil, false, true, errs
}

if _, err := c.secretLister.Secrets("openshift-authentication").Get("v4-0-config-system-custom-router-certs"); err == nil {
Expand All @@ -213,13 +227,13 @@ func (c *oauthServerDeploymentSyncer) Sync(ctx context.Context, syncContext fact

err = c.ensureAtMostOnePodPerNode(&expectedDeployment.Spec, "oauth-openshift")
if err != nil {
return nil, false, append(errs, fmt.Errorf("unable to ensure at most one pod per node: %v", err))
return nil, false, false, append(errs, fmt.Errorf("unable to ensure at most one pod per node: %v", err))
}

// Set the replica count to the number of master nodes.
masterNodeCount, err := c.countNodes(expectedDeployment.Spec.Template.Spec.NodeSelector)
if err != nil {
return nil, false, append(errs, fmt.Errorf("failed to determine number of master nodes: %v", err))
return nil, false, false, append(errs, fmt.Errorf("failed to determine number of master nodes: %v", err))
}
expectedDeployment.Spec.Replicas = masterNodeCount

Expand All @@ -229,10 +243,10 @@ func (c *oauthServerDeploymentSyncer) Sync(ctx context.Context, syncContext fact
resourcemerge.ExpectedDeploymentGeneration(expectedDeployment, operatorStatus.Generations),
)
if err != nil {
return nil, false, append(errs, fmt.Errorf("applying deployment of the integrated OAuth server failed: %w", err))
return nil, false, false, append(errs, fmt.Errorf("applying deployment of the integrated OAuth server failed: %w", err))
}

return deployment, true, errs
return deployment, true, false, errs
}

func (c *oauthServerDeploymentSyncer) getProxyConfig() (*configv1.Proxy, error) {
Expand Down Expand Up @@ -274,3 +288,13 @@ func (c *oauthServerDeploymentSyncer) getConfigResourceVersions() ([]string, err

return configRVs, nil
}

func (c *oauthServerDeploymentSyncer) oidcEnabled() (bool, error) {
auth, err := c.authLister.Get("cluster")
if err != nil {
return false, err
}

// note that the actual check will be more involved than this; this is just for demo purposes
return auth.Spec.Type == configv1.AuthenticationTypeOIDC, nil
}
3 changes: 2 additions & 1 deletion pkg/operator/starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ func prepareOauthOperator(
authOperatorInput.configClient.ConfigV1().ClusterOperators(),
informerFactories.operatorConfigInformer,
informerFactories.namespacedOpenshiftAuthenticationRoutes,
informerFactories.operatorConfigInformer.Config().V1().Authentications().Lister(),
bootstrapauthenticator.NewBootstrapUserDataGetter(authOperatorInput.kubeClient.CoreV1(), authOperatorInput.kubeClient.CoreV1()),
authOperatorInput.eventRecorder,
versionRecorder,
Expand Down Expand Up @@ -416,6 +417,7 @@ func prepareOauthAPIServerOperator(
os.Getenv("IMAGE_OAUTH_APISERVER"),
os.Getenv("OPERATOR_IMAGE"),
authOperatorInput.kubeClient,
informerFactories.operatorConfigInformer.Config().V1().Authentications().Lister(),
versionRecorder)

infra, err := authOperatorInput.configClient.ConfigV1().Infrastructures().Get(ctx, "cluster", metav1.GetOptions{})
Expand Down Expand Up @@ -551,7 +553,6 @@ func prepareOauthAPIServerOperator(
).WithAuditPolicyController(
"openshift-oauth-apiserver",
"audit",
informerFactories.operatorConfigInformer.Config().V1().APIServers().Lister(),
informerFactories.operatorConfigInformer,
informerFactories.kubeInformersForNamespaces.InformersFor("openshift-oauth-apiserver"),
authOperatorInput.kubeClient,
Expand Down
51 changes: 37 additions & 14 deletions pkg/operator/workload/sync_openshift_oauth_apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ import (
"github.com/openshift/library-go/pkg/operator/v1helpers"

appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/kubernetes"
"k8s.io/klog/v2"

configv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"
configv1listers "github.com/openshift/client-go/config/listers/config/v1"
"github.com/openshift/library-go/pkg/controller/factory"
libgoetcd "github.com/openshift/library-go/pkg/operator/configobserver/etcd"
"github.com/openshift/library-go/pkg/operator/events"
Expand Down Expand Up @@ -51,6 +54,7 @@ type OAuthAPIServerWorkload struct {
operatorImagePullSpec string
kubeClient kubernetes.Interface
versionRecorder status.VersionGetter
authLister configv1listers.AuthenticationLister
}

// NewOAuthAPIServerWorkload creates new OAuthAPIServerWorkload struct
Expand All @@ -62,6 +66,7 @@ func NewOAuthAPIServerWorkload(
targetImagePullSpec string,
operatorImagePullSpec string,
kubeClient kubernetes.Interface,
authLister configv1listers.AuthenticationLister,
versionRecorder status.VersionGetter,
) *OAuthAPIServerWorkload {
return &OAuthAPIServerWorkload{
Expand All @@ -72,6 +77,7 @@ func NewOAuthAPIServerWorkload(
targetImagePullSpec: targetImagePullSpec,
operatorImagePullSpec: operatorImagePullSpec,
kubeClient: kubeClient,
authLister: authLister,
versionRecorder: versionRecorder,
}
}
Expand Down Expand Up @@ -117,41 +123,41 @@ func (c *OAuthAPIServerWorkload) preconditionFulfilledInternal(operatorSpec *ope
}

// Sync essentially manages OAuthAPI server.
func (c *OAuthAPIServerWorkload) Sync(ctx context.Context, syncCtx factory.SyncContext) (*appsv1.Deployment, bool, []error) {
func (c *OAuthAPIServerWorkload) Sync(ctx context.Context, syncCtx factory.SyncContext) (*appsv1.Deployment, bool, bool, []error) {
errs := []error{}

operatorSpec, operatorStatus, _, err := c.operatorClient.GetOperatorState()
if err != nil {
errs = append(errs, err)
return nil, false, errs
return nil, false, false, errs
}

actualDeployment, err := c.syncDeployment(ctx, operatorSpec, operatorStatus, syncCtx.Recorder())
actualDeployment, deploymentDeleted, err := c.syncDeployment(ctx, operatorSpec, operatorStatus, syncCtx.Recorder())
if err != nil {
errs = append(errs, fmt.Errorf("%q: %v", "deployments", err))
}
return actualDeployment, true, errs
return actualDeployment, true, deploymentDeleted, errs
}

func (c *OAuthAPIServerWorkload) syncDeployment(ctx context.Context, operatorSpec *operatorv1.OperatorSpec, operatorStatus *operatorv1.OperatorStatus, eventRecorder events.Recorder) (*appsv1.Deployment, error) {
func (c *OAuthAPIServerWorkload) syncDeployment(ctx context.Context, operatorSpec *operatorv1.OperatorSpec, operatorStatus *operatorv1.OperatorStatus, eventRecorder events.Recorder) (*appsv1.Deployment, bool, error) {
if operatorStatus.LatestAvailableRevision == 0 {
// this a backstop during the migration from 4.17 whe this information is in .status.oauthAPIServer.latestAvailableRevision
return nil, fmt.Errorf(".status.latestAvailableRevision is not yet available")
return nil, false, fmt.Errorf(".status.latestAvailableRevision is not yet available")
}

tmpl, err := bindata.Asset("oauth-apiserver/deploy.yaml")
if err != nil {
return nil, err
return nil, false, err
}

argsRaw, err := GetAPIServerArgumentsRaw(*operatorSpec)
if err != nil {
return nil, err
return nil, false, err
}

args, err := arguments.Parse(argsRaw)
if err != nil {
return nil, err
return nil, false, err
}

// log level verbosity is taken from the spec always
Expand All @@ -167,11 +173,18 @@ func (c *OAuthAPIServerWorkload) syncDeployment(ctx context.Context, operatorSpe
tmpl = []byte(r.Replace(string(tmpl)))
re := regexp.MustCompile(`\$\{[^}]*}`)
if match := re.Find(tmpl); len(match) > 0 && !excludedReferences.Has(string(match)) {
return nil, fmt.Errorf("invalid template reference %q", string(match))
return nil, false, fmt.Errorf("invalid template reference %q", string(match))
}

required := resourceread.ReadDeploymentV1OrDie(tmpl)

if oidcEnabled, err := c.oidcEnabled(); err != nil {
return nil, false, err
} else if oidcEnabled {
err := c.kubeClient.AppsV1().Deployments(required.Namespace).Delete(ctx, required.Name, metav1.DeleteOptions{})
return nil, err == nil, err
}

// use the following routine for things that would require special formatting/padding (yaml)
encodedArgs := arguments.EncodeWithDelimiter(args, " \\\n ")
r = strings.NewReplacer(
Expand Down Expand Up @@ -208,7 +221,7 @@ func (c *OAuthAPIServerWorkload) syncDeployment(ctx context.Context, operatorSpe
resourcehash.NewObjectRef().ForConfigMap().InNamespace(c.targetNamespace).Named("trusted-ca-bundle"),
)
if err != nil {
return nil, fmt.Errorf("invalid dependency reference: %q", err)
return nil, false, fmt.Errorf("invalid dependency reference: %q", err)
}

for k, v := range inputHashes {
Expand All @@ -222,18 +235,28 @@ func (c *OAuthAPIServerWorkload) syncDeployment(ctx context.Context, operatorSpe

err = c.ensureAtMostOnePodPerNode(&required.Spec, "oauth-apiserver")
if err != nil {
return nil, fmt.Errorf("unable to ensure at most one pod per node: %v", err)
return nil, false, fmt.Errorf("unable to ensure at most one pod per node: %v", err)
}

// Set the replica count to the number of master nodes.
masterNodeCount, err := c.countNodes(required.Spec.Template.Spec.NodeSelector)
if err != nil {
return nil, fmt.Errorf("failed to determine number of master nodes: %v", err)
return nil, false, fmt.Errorf("failed to determine number of master nodes: %v", err)
}
required.Spec.Replicas = masterNodeCount

deployment, _, err := resourceapply.ApplyDeployment(ctx, c.kubeClient.AppsV1(), eventRecorder, required, resourcemerge.ExpectedDeploymentGeneration(required, operatorStatus.Generations))
return deployment, err
return deployment, false, err
}

func (c *OAuthAPIServerWorkload) oidcEnabled() (bool, error) {
auth, err := c.authLister.Get("cluster")
if err != nil {
return false, err
}

// note that the actual check will be more involved than this; this is just for demo purposes
return auth.Spec.Type == configv1.AuthenticationTypeOIDC, nil
}

func loglevelToKlog(logLevel operatorv1.LogLevel) string {
Expand Down
Loading