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

OCPBUGS-16614: *: stop checking for the STS feature gate #579

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions go.mod
Expand Up @@ -64,7 +64,6 @@ require (
github.com/google/go-cmp v0.5.9
github.com/microsoft/kiota-authentication-azure-go v0.6.0
github.com/microsoftgraph/msgraph-sdk-go v0.59.0
github.com/openshift/client-go v0.0.0-20230503144108-75015d2347cb
k8s.io/klog/v2 v2.90.1
sigs.k8s.io/e2e-framework v0.2.0
)
Expand Down Expand Up @@ -148,7 +147,6 @@ require (
gopkg.in/ini.v1 v1.62.0 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
k8s.io/apiextensions-apiserver v0.27.2 // indirect
k8s.io/apiserver v0.27.2 // indirect
k8s.io/component-base v0.27.2 // indirect
k8s.io/gengo v0.0.0-20220902162205-c0856e24416d // indirect
k8s.io/kube-aggregator v0.27.1 // indirect
Expand Down
4 changes: 0 additions & 4 deletions go.sum
Expand Up @@ -625,8 +625,6 @@ github.com/openshift/api v0.0.0-20230717193525-8ad30b402d92 h1:keFXCu/8bfY3QUOc1
github.com/openshift/api v0.0.0-20230717193525-8ad30b402d92/go.mod h1:yimSGmjsI+XF1mr+AKBs2//fSXIOhhetHGbMlBEfXbs=
github.com/openshift/build-machinery-go v0.0.0-20230306181456-d321ffa04533 h1:mh3ZYs7kPIIe3UUY6tJcTExmtjnXXUu0MrBuK2W/Qvw=
github.com/openshift/build-machinery-go v0.0.0-20230306181456-d321ffa04533/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
github.com/openshift/client-go v0.0.0-20230503144108-75015d2347cb h1:Nij5OnaECrkmcRQMAE9LMbQXPo95aqFnf+12B7SyFVI=
github.com/openshift/client-go v0.0.0-20230503144108-75015d2347cb/go.mod h1:Rhb3moCqeiTuGHAbXBOlwPubUMlOZEkrEWTRjIF3jzs=
github.com/openshift/library-go v0.0.0-20230620084201-504ca4bd5a83 h1:z7tTnbZ2bzPtXjVnWHWCtUCBYrZYeKJitkV1rffmMY8=
github.com/openshift/library-go v0.0.0-20230620084201-504ca4bd5a83/go.mod h1:PegtilvJPBJXjJG3AV8uL1a0SAnBr6K67ShNiWVb40M=
github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=
Expand Down Expand Up @@ -1323,8 +1321,6 @@ k8s.io/apimachinery v0.27.3 h1:Ubye8oBufD04l9QnNtW05idcOe9Z3GQN8+7PqmuVcUM=
k8s.io/apimachinery v0.27.3/go.mod h1:XNfZ6xklnMCOGGFNqXG7bUrQCoR04dh/E7FprV6pb+E=
k8s.io/apiserver v0.17.0/go.mod h1:ABM+9x/prjINN6iiffRVNCBR2Wk7uY4z+EtEGZD48cg=
k8s.io/apiserver v0.18.0-beta.2/go.mod h1:bnblMkMoCFnIfVnVftd0SXJPzyvrk3RtaqSbblphF/A=
k8s.io/apiserver v0.27.2 h1:p+tjwrcQEZDrEorCZV2/qE8osGTINPuS5ZNqWAvKm5E=
k8s.io/apiserver v0.27.2/go.mod h1:EsOf39d75rMivgvvwjJ3OW/u9n1/BmUMK5otEOJrb1Y=
k8s.io/client-go v0.17.0/go.mod h1:TYgR6EUHs6k45hb6KWjVD6jFZvJV4gHDikv/It0xz+k=
k8s.io/client-go v0.18.0-beta.2/go.mod h1:UvuVxHjKWIcgy0iMvF+bwNDW7l0mskTNOaOW1Qv5BMA=
k8s.io/client-go v0.24.2/go.mod h1:zg4Xaoo+umDsfCWr4fCnmLEtQXyCNXCvJuSsglNcV30=
Expand Down
41 changes: 15 additions & 26 deletions pkg/aws/actuator/actuator.go
Expand Up @@ -69,23 +69,21 @@ var _ actuatoriface.Actuator = (*AWSActuator)(nil)

// AWSActuator implements the CredentialsRequest Actuator interface to create credentials in AWS.
type AWSActuator struct {
Client client.Client
RootCredClient client.Client
LiveClient client.Client
AWSClientBuilder func(accessKeyID, secretAccessKey []byte, c client.Client) (ccaws.Client, error)
Scheme *runtime.Scheme
AWSSecurityTokenServiceGateEnabled bool
Client client.Client
RootCredClient client.Client
LiveClient client.Client
AWSClientBuilder func(accessKeyID, secretAccessKey []byte, c client.Client) (ccaws.Client, error)
Scheme *runtime.Scheme
}

// NewAWSActuator creates a new AWSActuator.
func NewAWSActuator(client, rootCredClient, liveClient client.Client, scheme *runtime.Scheme, awsSecurityTokenServiceGateEnabled bool) (*AWSActuator, error) {
func NewAWSActuator(client, rootCredClient, liveClient client.Client, scheme *runtime.Scheme) (*AWSActuator, error) {
return &AWSActuator{
Client: client,
LiveClient: liveClient,
RootCredClient: rootCredClient,
AWSClientBuilder: awsutils.ClientBuilder,
Scheme: scheme,
AWSSecurityTokenServiceGateEnabled: awsSecurityTokenServiceGateEnabled,
Client: client,
LiveClient: liveClient,
RootCredClient: rootCredClient,
AWSClientBuilder: awsutils.ClientBuilder,
Scheme: scheme,
}, nil
}

Expand Down Expand Up @@ -116,10 +114,6 @@ func DecodeProviderSpec(codec *minterv1.ProviderCodec, cr *minterv1.CredentialsR
return nil, fmt.Errorf("no providerSpec defined")
}

func (a *AWSActuator) STSFeatureGateEnabled() bool {
return a.AWSSecurityTokenServiceGateEnabled
}

// Checks if the credentials currently exist.
//
// To do this we will check if the target secret exists. This call is only used to determine
Expand Down Expand Up @@ -334,17 +328,12 @@ func (a *AWSActuator) sync(ctx context.Context, cr *minterv1.CredentialsRequest)
logger.Debug("credentials already up to date")
return nil
}
stsDetected := false
stsFeatureGateEnabled := a.STSFeatureGateEnabled()
if stsFeatureGateEnabled {
stsDetected, err = utils.IsTimedTokenCluster(a.Client, ctx, logger)
if err != nil {
return err
}
stsDetected, err := utils.IsTimedTokenCluster(a.Client, ctx, logger)
if err != nil {
return err
}
logger.Infof("stsFeatureGateEnabled: %v", stsFeatureGateEnabled)
logger.Infof("stsDetected: %v", stsDetected)
if stsFeatureGateEnabled && stsDetected {
if stsDetected {
logger.Debug("actuator detected STS enabled cluster, enabling STS secret brokering for CredentialsRequests providing an IAM Role ARN")
awsSTSIAMRoleARN, err := awsSTSIAMRoleARN(minterv1.Codec, cr)
if err != nil {
Expand Down
16 changes: 6 additions & 10 deletions pkg/aws/actuator/actuator_test.go
Expand Up @@ -577,7 +577,6 @@ func TestDetectSTS(t *testing.T) {
wantErr assert.ErrorAssertionFunc
CredentialsRequest *minterv1.CredentialsRequest
issuer string
stsEnabled bool
}{
{
name: "empty ServiceAccountIssuer on AWS STS-enabled CCO in Manual mode should error",
Expand Down Expand Up @@ -614,9 +613,8 @@ func TestDetectSTS(t *testing.T) {
}
return cr
}(),
issuer: "non-empty",
stsEnabled: true,
wantErr: assert.NoError,
issuer: "non-empty",
wantErr: assert.NoError,
},
{
name: "STS mode and with a CloudTokenString and CloudTokenPath set in CredentialsRequest should create Secret & not error",
Expand All @@ -634,9 +632,8 @@ func TestDetectSTS(t *testing.T) {
cr.Spec.CloudTokenPath = "/var/token"
return cr
}(),
issuer: "non-empty",
stsEnabled: true,
wantErr: assert.NoError,
issuer: "non-empty",
wantErr: assert.NoError,
},
}
for _, test := range tests {
Expand All @@ -651,9 +648,8 @@ func TestDetectSTS(t *testing.T) {
panic(err)
}
a := &AWSActuator{
Client: fakeClient,
RootCredClient: fakeAdminClient,
AWSSecurityTokenServiceGateEnabled: test.stsEnabled,
Client: fakeClient,
RootCredClient: fakeAdminClient,
}
test.wantErr(t, a.sync(context.Background(), test.CredentialsRequest), fmt.Sprintf("sync(%v)", test.CredentialsRequest))
})
Expand Down
4 changes: 0 additions & 4 deletions pkg/azure/actuator.go
Expand Up @@ -49,10 +49,6 @@ type Actuator struct {
credentialMinterBuilder credentialMinterBuilder
}

func (a *Actuator) STSFeatureGateEnabled() bool {
return false
}

func NewActuator(c, rootCredClient client.Client, cloudName configv1.AzureCloudEnvironment) (*Actuator, error) {
client := newClientWrapper(c, rootCredClient)
return &Actuator{
Expand Down
8 changes: 1 addition & 7 deletions pkg/cmd/operator/cmd.go
Expand Up @@ -106,12 +106,6 @@ func NewOperator() *cobra.Command {
ctrlruntimelog.SetLogger(logr.New(ctrlruntimelog.NullLogSink{}))

log.Info("checking prerequisites")
featureGates, err := platform.GetFeatureGates(ctx)
if err != nil {
log.WithError(err).Fatal("unable to read feature gates")
}
awsSecurityTokenServiceGateEnabled := featureGates.Enabled(configv1.FeatureGateAWSSecurityTokenService)

kubeconfigCommandLinePath := cmd.PersistentFlags().Lookup("kubeconfig").Value.String()
rules := clientcmd.NewDefaultClientConfigLoadingRules()
rules.ExplicitPath = kubeconfigCommandLinePath
Expand Down Expand Up @@ -213,7 +207,7 @@ func NewOperator() *cobra.Command {

// Setup all Controllers
log.Info("setting up controllers")
if err := controller.AddToManager(mgr, rootMgr, kubeconfigCommandLinePath, coreClient, awsSecurityTokenServiceGateEnabled); err != nil {
if err := controller.AddToManager(mgr, rootMgr, kubeconfigCommandLinePath, coreClient); err != nil {
log.WithError(err).Fatal("unable to register controllers to the manager")
}

Expand Down
4 changes: 0 additions & 4 deletions pkg/gcp/actuator/actuator.go
Expand Up @@ -65,10 +65,6 @@ type Actuator struct {
GCPClientBuilder func(string, []byte) (ccgcp.Client, error)
}

func (a *Actuator) STSFeatureGateEnabled() bool {
return false
}

// NewActuator initializes and returns a new Actuator for GCP.
func NewActuator(c, rootCredClient client.Client, projectName string) (*Actuator, error) {
return &Actuator{
Expand Down
4 changes: 0 additions & 4 deletions pkg/kubevirt/actuator.go
Expand Up @@ -41,10 +41,6 @@ type KubevirtActuator struct {
RootCredClient client.Client
}

func (a *KubevirtActuator) STSFeatureGateEnabled() bool {
return false
}

const (
KubevirtCredentialsSecretKey = "kubeconfig"
)
Expand Down
4 changes: 0 additions & 4 deletions pkg/openstack/actuator.go
Expand Up @@ -42,10 +42,6 @@ type OpenStackActuator struct {
RootCredClient client.Client
}

func (a *OpenStackActuator) STSFeatureGateEnabled() bool {
return false
}

// NewOpenStackActuator creates a new OpenStack actuator.
func NewOpenStackActuator(client, rootCredClient client.Client) (*OpenStackActuator, error) {
return &OpenStackActuator{
Expand Down
4 changes: 2 additions & 2 deletions pkg/operator/controller.go
Expand Up @@ -66,7 +66,7 @@ var AddToManagerFuncs []func(manager.Manager, manager.Manager, string) error
var AddToManagerWithActuatorFuncs []func(manager.Manager, manager.Manager, actuator.Actuator, configv1.PlatformType, corev1client.CoreV1Interface) error

// AddToManager adds all Controllers to the Manager
func AddToManager(m, rootM manager.Manager, explicitKubeconfig string, coreClient corev1client.CoreV1Interface, awsSecurityTokenServiceGateEnabled bool) error {
func AddToManager(m, rootM manager.Manager, explicitKubeconfig string, coreClient corev1client.CoreV1Interface) error {
for _, f := range AddToManagerFuncs {
if err := f(m, rootM, explicitKubeconfig); err != nil {
return err
Expand All @@ -86,7 +86,7 @@ func AddToManager(m, rootM manager.Manager, explicitKubeconfig string, coreClien
switch platformType {
case configv1.AWSPlatformType:
log.Info("initializing AWS actuator")
a, err = awsactuator.NewAWSActuator(m.GetClient(), rootM.GetClient(), utils.LiveClient(m), m.GetScheme(), awsSecurityTokenServiceGateEnabled)
a, err = awsactuator.NewAWSActuator(m.GetClient(), rootM.GetClient(), utils.LiveClient(m), m.GetScheme())
if err != nil {
return err
}
Expand Down
6 changes: 0 additions & 6 deletions pkg/operator/credentialsrequest/actuator/actuator.go
Expand Up @@ -46,8 +46,6 @@ type Actuator interface {
Upgradeable(operatorv1.CloudCredentialsMode) *configv1.ClusterOperatorStatusCondition
// GetCredentialsRootSecret returns the credentials root secret.
GetCredentialsRootSecret(ctx context.Context, cr *minterv1.CredentialsRequest) (*corev1.Secret, error)
// Query STSFeatureGateEnabled.
STSFeatureGateEnabled() bool
}

type DummyActuator struct {
Expand Down Expand Up @@ -86,10 +84,6 @@ func (a *DummyActuator) GetCredentialsRootSecret(ctx context.Context, cr *minter
return nil, nil
}

func (a *DummyActuator) STSFeatureGateEnabled() bool {
return false
}

type ActuatorError struct {
ErrReason minterv1.CredentialsRequestConditionType
Message string
Expand Down
13 changes: 5 additions & 8 deletions pkg/operator/credentialsrequest/credentialsrequest_controller.go
Expand Up @@ -19,11 +19,12 @@ package credentialsrequest
import (
"context"
"fmt"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/util/retry"
"reflect"
"time"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/util/retry"

log "github.com/sirupsen/logrus"
"golang.org/x/time/rate"
corev1applyconfigurations "k8s.io/client-go/applyconfigurations/core/v1"
Expand Down Expand Up @@ -518,11 +519,7 @@ func (r *ReconcileCredentialsRequest) Reconcile(ctx context.Context, request rec
mode, conflict, err := utils.GetOperatorConfiguration(r.Client, logger)

stsDetected := false
stsFeatureGateEnabled := r.Actuator.STSFeatureGateEnabled()

if stsFeatureGateEnabled {
stsDetected, _ = utils.IsTimedTokenCluster(r.Client, ctx, logger)
}
stsDetected, _ = utils.IsTimedTokenCluster(r.Client, ctx, logger)
if err != nil {
logger.WithError(err).Error("error checking if operator is disabled")
return reconcile.Result{}, err
Expand Down Expand Up @@ -679,7 +676,7 @@ func (r *ReconcileCredentialsRequest) Reconcile(ctx context.Context, request rec
} else {
crSecretExists = true
}
if stsFeatureGateEnabled && stsDetected {
if stsDetected {
// create time-based tokens based on settings in CredentialsRequests
logger.Debugf("timed token access cluster detected: %t, so not trying to provision with root secret",
stsDetected)
Expand Down
74 changes: 0 additions & 74 deletions pkg/operator/platform/platform.go
Expand Up @@ -2,26 +2,13 @@ package platform

import (
"context"
"fmt"
"os"
"time"

"github.com/openshift/cloud-credential-operator/pkg/operator/constants"
"github.com/openshift/cloud-credential-operator/pkg/util"

log "github.com/sirupsen/logrus"

configv1 "github.com/openshift/api/config/v1"
configclient "github.com/openshift/client-go/config/clientset/versioned"
configinformers "github.com/openshift/client-go/config/informers/externalversions"
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
"github.com/openshift/library-go/pkg/operator/events"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/clientcmd"
"sigs.k8s.io/controller-runtime/pkg/client"
crtconfig "sigs.k8s.io/controller-runtime/pkg/client/config"
)

// GetInfraStatusUsingKubeconfig queries the k8s api for the infrastructure CR using the kubeconfig file
Expand Down Expand Up @@ -75,64 +62,3 @@ func getClient(explicitKubeconfig string) (client.Client, error) {

return dynamicClient, nil
}

func GetFeatureGates(ctx context.Context) (featuregates.FeatureGate, error) {
stevekuznetsov marked this conversation as resolved.
Show resolved Hide resolved
stop := make(chan struct{})
ctx, cancelFn := context.WithCancel(ctx)
go func() {
defer cancelFn()
<-stop
}()

config, err := crtconfig.GetConfig()
if err != nil {
return nil, fmt.Errorf("failed to get kube config: %v", err)
}
clientSet, err := configclient.NewForConfig(config)
if err != nil {
return nil, err
}
configInformers := configinformers.NewSharedInformerFactory(clientSet, 10*time.Minute)
desiredVersion := computeClusterOperatorVersions()
missingVersion := desiredVersion

kubeClient, err := kubernetes.NewForConfig(config)
if err != nil {
return nil, fmt.Errorf("failed to create kube client: %w", err)
}
eventRecorder := events.NewKubeRecorder(kubeClient.CoreV1().Events("openshift-cloud-credential-operator"), "cloud-credential-operator", &corev1.ObjectReference{
APIVersion: "apps/v1",
Kind: "Deployment",
Namespace: constants.CCONameSpace,
Name: constants.DeploymentName,
})

// By default, this will exit(0) the process if the featuregates ever change to a different set of values.
featureGateAccessor := featuregates.NewFeatureGateAccess(
desiredVersion, missingVersion,
configInformers.Config().V1().ClusterVersions(), configInformers.Config().V1().FeatureGates(),
eventRecorder,
)
go featureGateAccessor.Run(ctx)
go configInformers.Start(stop)

select {
case <-featureGateAccessor.InitialFeatureGatesObserved():
featureGates, _ := featureGateAccessor.CurrentFeatureGates()
log.Info("FeatureGates initialized", "knownFeatures", featureGates.KnownFeatures())
case <-time.After(1 * time.Minute):
log.Error(nil, "timed out waiting for FeatureGate detection")
return nil, fmt.Errorf("timed out waiting for FeatureGate detection")
}

featureGates, err := featureGateAccessor.CurrentFeatureGates()
if err != nil {
return nil, err
}
return featureGates, nil
}

func computeClusterOperatorVersions() string {
currentVersion := os.Getenv("RELEASE_VERSION")
return currentVersion
}