Skip to content

Refactor some things #2775

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.

Sign up for GitHub

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

Merged
openshift-merge-bot merged 1 commit into from
Nov 6, 2025
Merged

Refactor some things #2775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 0 additions & 2 deletions cmd/manager/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"flag"
golog "log"
"math/rand"
"net/http"
_ "net/http/pprof"
"os"
Expand Down Expand Up @@ -258,7 +257,6 @@ func main() {
log.WithField("pprof_host_port", pprofHostPort).Info("Enabling pprof")
log.Println(http.ListenAndServe(pprofHostPort, nil))
}()
rand.Seed(time.Now().UnixNano())
Copy link
Member Author

@2uasimojo 2uasimojo Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deprecated/obsolete/no-op

Copy link
Contributor

@suhanime suhanime Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was interesting to learn why this was done in the first place - glad it is no longer needed.

cmd := newRootCommand()
err := cmd.Execute()
if err != nil {
Expand Down
35 changes: 17 additions & 18 deletions contrib/pkg/adm/managedns/enable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/config"

hivev1 "github.com/openshift/hive/apis/hive/v1"
hiveutils "github.com/openshift/hive/contrib/pkg/utils"
awsutils "github.com/openshift/hive/contrib/pkg/utils/aws"
azureutils "github.com/openshift/hive/contrib/pkg/utils/azure"
gcputils "github.com/openshift/hive/contrib/pkg/utils/gcp"
"github.com/openshift/hive/contrib/pkg/utils"
"github.com/openshift/hive/pkg/constants"
awscreds "github.com/openshift/hive/pkg/creds/aws"
azurecreds "github.com/openshift/hive/pkg/creds/azure"
gcpcreds "github.com/openshift/hive/pkg/creds/gcp"
"github.com/openshift/hive/pkg/resource"
"github.com/openshift/hive/pkg/util/scheme"
)
Expand All @@ -46,9 +46,6 @@ managed domains, create a credentials secret for your cloud provider, and link i
the ExternalDNS section of HiveConfig.
`
const (
cloudAWS = "aws"
cloudGCP = "gcp"
cloudAzure = "azure"
Comment on lines -49 to -51
Copy link
Member Author

@2uasimojo 2uasimojo Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use global consts

hiveAdmissionDeployment = "hiveadmission"
hiveConfigName = "hive"
waitTime = time.Minute * 2
Expand Down Expand Up @@ -95,7 +92,7 @@ func NewEnableManageDNSCommand() *cobra.Command {
}

flags := cmd.Flags()
flags.StringVar(&opt.Cloud, "cloud", cloudAWS, "Cloud provider: aws(default)|gcp|azure)")
flags.StringVar(&opt.Cloud, "cloud", constants.PlatformAWS, "Cloud provider: aws(default)|gcp|azure)")
flags.StringVar(&opt.CredsFile, "creds-file", "", "Cloud credentials file (defaults vary depending on cloud)")
flags.StringVar(&opt.AzureResourceGroup, "azure-resource-group-name", "os4-common", "Azure Resource Group (Only applicable if --cloud azure)")
return cmd
Expand Down Expand Up @@ -127,8 +124,7 @@ func (o *Options) Run(args []string) error {
// Update the current HiveConfig, which should always exist as the operator will
// create a default one once run.
hc := &hivev1.HiveConfig{}
o.hiveClient.Get(context.TODO(), types.NamespacedName{Name: hiveConfigName}, hc)
if err != nil {
Copy link
Member Author

@2uasimojo 2uasimojo Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was checking the wrong err (which couldn't be nil if we got here).

if err := o.hiveClient.Get(context.TODO(), types.NamespacedName{Name: hiveConfigName}, hc); err != nil {
log.WithError(err).Fatal("error looking up HiveConfig 'hive'")
}

Expand All @@ -139,7 +135,7 @@ func (o *Options) Run(args []string) error {
var credsSecret *corev1.Secret

switch o.Cloud {
case cloudAWS:
case constants.PlatformAWS:
// Apply a secret for credentials to manage the root domain:
credsSecret, err = o.generateAWSCredentialsSecret()
if err != nil {
Expand All @@ -148,7 +144,7 @@ func (o *Options) Run(args []string) error {
dnsConf.AWS = &hivev1.ManageDNSAWSConfig{
CredentialsSecretRef: corev1.LocalObjectReference{Name: credsSecret.Name},
}
case cloudGCP:
case constants.PlatformGCP:
// Apply a secret for credentials to manage the root domain:
credsSecret, err = o.generateGCPCredentialsSecret()
if err != nil {
Expand All @@ -157,7 +153,7 @@ func (o *Options) Run(args []string) error {
dnsConf.GCP = &hivev1.ManageDNSGCPConfig{
CredentialsSecretRef: corev1.LocalObjectReference{Name: credsSecret.Name},
}
case cloudAzure:
case constants.PlatformAzure:
credsSecret, err = o.generateAzureCredentialsSecret()
if err != nil {
log.WithError(err).Fatal("error generating manageDNS credentials secret")
Expand Down Expand Up @@ -313,7 +309,7 @@ func (o *Options) waitForHiveConfigToBeProcessed() error {

func (o *Options) generateAWSCredentialsSecret() (*corev1.Secret, error) {
defaultCredsFilePath := filepath.Join(o.homeDir, ".aws", "credentials")
accessKeyID, secretAccessKey, err := awsutils.GetAWSCreds(o.CredsFile, defaultCredsFilePath)
accessKeyID, secretAccessKey, err := awscreds.GetAWSCreds(o.CredsFile, defaultCredsFilePath)
if err != nil {
return nil, err
}
Expand All @@ -334,7 +330,7 @@ func (o *Options) generateAWSCredentialsSecret() (*corev1.Secret, error) {
}

func (o *Options) generateGCPCredentialsSecret() (*corev1.Secret, error) {
saFileContents, err := gcputils.GetCreds(o.CredsFile)
saFileContents, err := gcpcreds.GetCreds(o.CredsFile)
if err != nil {
return nil, err
}
Expand All @@ -354,7 +350,7 @@ func (o *Options) generateGCPCredentialsSecret() (*corev1.Secret, error) {
}

func (o *Options) generateAzureCredentialsSecret() (*corev1.Secret, error) {
spFileContents, err := azureutils.GetCreds(o.CredsFile)
spFileContents, err := azurecreds.GetCreds(o.CredsFile)
if err != nil {
return nil, err
}
Expand All @@ -379,12 +375,15 @@ func (o *Options) getResourceHelper() (resource.Helper, error) {
log.WithError(err).Error("Cannot get client config")
return nil, err
}
return resource.NewHelperFromRESTConfig(cfg, "util-managedns-enable", log.WithField("command", "adm manage-dns enable"))
return resource.NewHelper(
log.WithField("command", "adm manage-dns enable"),
resource.FromRESTConfig(cfg),
resource.WithControllerName("util-managedns-enable"))
}

func (o *Options) setupLocalClients() error {
log.Debug("creating cluster client config")
hiveClient, err := hiveutils.GetClient("hiveutil-managedns-enable")
hiveClient, err := utils.GetClient("hiveutil-managedns-enable")
if err != nil {
log.WithError(err).Error("failed to create a hive config client")
return err
Expand Down
11 changes: 11 additions & 0 deletions contrib/pkg/awsprivatelink/awsprivatelink.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,17 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
)

const (
// privateLinkHubAcctCredsName is the name of the AWS PrivateLink Hub account credentials Secret
// created by the "hiveutil awsprivatelink enable" command
privateLinkHubAcctCredsName = "awsprivatelink-hub-acct-creds"

// privateLinkHubAcctCredsLabel is added to the AWS PrivateLink Hub account credentials Secret
// created by the "hiveutil awsprivatelink enable" command and
// referenced by HiveConfig.spec.awsPrivateLink.credentialsSecretRef.
privateLinkHubAcctCredsLabel = "hive.openshift.io/awsprivatelink-hub-acct-credentials"
)
Comment on lines +20 to +29
Copy link
Member Author

@2uasimojo 2uasimojo Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only used in this package, so moved here from github.com/openshift/hive/contrib/pkg/utils/aws


var (
logLevelDebug bool
credsSecretRef string
Expand Down
5 changes: 2 additions & 3 deletions contrib/pkg/awsprivatelink/disable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (

hivev1 "github.com/openshift/hive/apis/hive/v1"
"github.com/openshift/hive/contrib/pkg/awsprivatelink/common"
awsutils "github.com/openshift/hive/contrib/pkg/utils/aws"
operatorutils "github.com/openshift/hive/pkg/operator/hive"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -78,8 +77,8 @@ func (o *disableOptions) Run(cmd *cobra.Command, args []string) error {
if err := common.DynamicClient.List(
context.Background(),
hubAcctSecrets,
client.MatchingFields{"metadata.name": awsutils.PrivateLinkHubAcctCredsName},
client.MatchingLabels{awsutils.PrivateLinkHubAcctCredsLabel: "true"},
client.MatchingFields{"metadata.name": privateLinkHubAcctCredsName},
client.MatchingLabels{privateLinkHubAcctCredsLabel: "true"},
client.InNamespace(hiveNS),
); err != nil {
log.WithError(err).Error("Failed to list Hub account credentials Secrets")
Expand Down
14 changes: 7 additions & 7 deletions contrib/pkg/awsprivatelink/enable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import (
configv1 "github.com/openshift/api/config/v1"
hivev1 "github.com/openshift/hive/apis/hive/v1"
"github.com/openshift/hive/contrib/pkg/awsprivatelink/common"
awsutils "github.com/openshift/hive/contrib/pkg/utils/aws"
"github.com/openshift/hive/pkg/awsclient"
awscreds "github.com/openshift/hive/pkg/creds/aws"
operatorutils "github.com/openshift/hive/pkg/operator/hive"

log "github.com/sirupsen/logrus"
Expand Down Expand Up @@ -147,11 +147,11 @@ func (o *enableOptions) Run(cmd *cobra.Command, args []string) error {

switch err = common.DynamicClient.Create(context.Background(), credsSecretInHiveNS); {
case err == nil:
log.Infof("Secret/%s created in namespace %s", awsutils.PrivateLinkHubAcctCredsName, hiveNS)
log.Infof("Secret/%s created in namespace %s", privateLinkHubAcctCredsName, hiveNS)
case apierrors.IsAlreadyExists(err):
log.Warnf("Secret/%s already exists in namespace %s", awsutils.PrivateLinkHubAcctCredsName, hiveNS)
log.Warnf("Secret/%s already exists in namespace %s", privateLinkHubAcctCredsName, hiveNS)
default:
log.WithError(err).Fatalf("Failed to create Secret/%s in namespace %s", awsutils.PrivateLinkHubAcctCredsName, hiveNS)
log.WithError(err).Fatalf("Failed to create Secret/%s in namespace %s", privateLinkHubAcctCredsName, hiveNS)
}

// Update HiveConfig
Expand Down Expand Up @@ -194,11 +194,11 @@ func (o *enableOptions) getOrCopyCredsSecret(source *corev1.Secret, namespace st
APIVersion: corev1.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: awsutils.PrivateLinkHubAcctCredsName,
Name: privateLinkHubAcctCredsName,
Namespace: namespace,
// Secrets without this label (e.g., the ones created and configured manually) won't be deleted
// when calling "hiveutil awsprivatelink disable".
Labels: map[string]string{awsutils.PrivateLinkHubAcctCredsLabel: "true"},
Labels: map[string]string{privateLinkHubAcctCredsLabel: "true"},
},
Type: corev1.SecretTypeOpaque,
}
Expand All @@ -210,7 +210,7 @@ func (o *enableOptions) getOrCopyCredsSecret(source *corev1.Secret, namespace st
// Get creds from environment
default:
defaultCredsFilePath := filepath.Join(o.homeDir, ".aws", "credentials")
accessKeyID, secretAccessKey, err := awsutils.GetAWSCreds("", defaultCredsFilePath)
accessKeyID, secretAccessKey, err := awscreds.GetAWSCreds("", defaultCredsFilePath)
if err != nil {
return nil, err
}
Expand Down
17 changes: 8 additions & 9 deletions contrib/pkg/awsprivatelink/endpointvpc/add.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (

hivev1 "github.com/openshift/hive/apis/hive/v1"
"github.com/openshift/hive/contrib/pkg/awsprivatelink/common"
awsutils "github.com/openshift/hive/contrib/pkg/utils/aws"
"github.com/openshift/hive/pkg/awsclient"

log "github.com/sirupsen/logrus"
Expand Down Expand Up @@ -94,7 +93,7 @@ func (o *endpointVPCAddOptions) Complete(cmd *cobra.Command, args []string) erro
regions.Insert(associatedVpc.AWSPrivateLinkVPC.Region)
}
// Use the passed-in credsSecret if possible
awsClientsByRegion, err := awsutils.GetAWSClientsByRegion(common.CredsSecret, regions)
awsClientsByRegion, err := getAWSClientsByRegion(common.CredsSecret, regions)
if err != nil {
log.WithError(err).Fatal("Failed to get AWS clients")
}
Expand Down Expand Up @@ -136,7 +135,7 @@ func (o *endpointVPCAddOptions) Validate(cmd *cobra.Command, args []string) erro

func (o *endpointVPCAddOptions) Run(cmd *cobra.Command, args []string) error {
// Get default SG of the endpoint VPC
endpointVPCDefaultSG, err := awsutils.GetDefaultSGOfVpc(o.endpointVpcClients, o.endpointVpcId)
endpointVPCDefaultSG, err := getDefaultSGOfVpc(o.endpointVpcClients, o.endpointVpcId)
if err != nil {
log.WithError(err).Fatal("Failed to get default SG of the endpoint VPC")
}
Expand Down Expand Up @@ -192,7 +191,7 @@ func (o *endpointVPCAddOptions) Run(cmd *cobra.Command, args []string) error {
}

// Update SGs
associatedVpcWorkerSG, err := awsutils.GetWorkerSGFromVpcId(associatedVpcClients, associatedVpcId)
associatedVpcWorkerSG, err := getWorkerSGFromVpcId(associatedVpcClients, associatedVpcId)
if err != nil {
log.WithError(err).Fatal("Failed to get worker SG of the associated VPC")
}
Expand All @@ -203,7 +202,7 @@ func (o *endpointVPCAddOptions) Run(cmd *cobra.Command, args []string) error {
// Associated VPC & endpoint VPC in the same region => allow ingress from SG of the peer
case associatedVpcRegion == o.endpointVpcRegion:
log.Info("Authorizing traffic from the associated VPC's worker SG to the endpoint VPC's default SG")
if _, err = awsutils.AuthorizeAllIngressFromSG(
if _, err = authorizeAllIngressFromSG(
o.endpointVpcClients,
aws.String(endpointVPCDefaultSG),
aws.String(associatedVpcWorkerSG),
Expand All @@ -218,7 +217,7 @@ func (o *endpointVPCAddOptions) Run(cmd *cobra.Command, args []string) error {
}

log.Info("Authorizing traffic from the endpoint VPC's default SG to the associated VPC's worker SG")
if _, err = awsutils.AuthorizeAllIngressFromSG(
if _, err = authorizeAllIngressFromSG(
associatedVpcClients,
aws.String(associatedVpcWorkerSG),
aws.String(endpointVPCDefaultSG),
Expand All @@ -235,7 +234,7 @@ func (o *endpointVPCAddOptions) Run(cmd *cobra.Command, args []string) error {
// Associated VPC & endpoint VPC in different regions => allow ingress from CIDR of the peer
default:
log.Info("Authorizing traffic from the associated VPC's CIDR block to the endpoint VPC's default SG")
if _, err = awsutils.AuthorizeAllIngressFromCIDR(
if _, err = authorizeAllIngressFromCIDR(
o.endpointVpcClients,
aws.String(endpointVPCDefaultSG),
associatedVpcCIDR,
Expand All @@ -250,7 +249,7 @@ func (o *endpointVPCAddOptions) Run(cmd *cobra.Command, args []string) error {
}

log.Info("Authorizing traffic from the endpoint VPC's CIDR block to the associated VPC's worker SG")
if _, err = awsutils.AuthorizeAllIngressFromCIDR(
if _, err = authorizeAllIngressFromCIDR(
associatedVpcClients,
aws.String(associatedVpcWorkerSG),
endpointVpcCIDR,
Expand Down Expand Up @@ -309,7 +308,7 @@ func (o *endpointVPCAddOptions) addEndpointVpcToHiveConfig() {
},
Subnets: endpointSubnets,
}
if idx, ok := awsutils.FindVpcInInventory(o.endpointVpcId, o.hiveConfig.Spec.AWSPrivateLink.EndpointVPCInventory); ok {
if idx, ok := findVpcInInventory(o.endpointVpcId, o.hiveConfig.Spec.AWSPrivateLink.EndpointVPCInventory); ok {
if reflect.DeepEqual(o.hiveConfig.Spec.AWSPrivateLink.EndpointVPCInventory[idx], endpointVpcToAdd) {
log.Warn("Endpoint VPC found in HiveConfig. HiveConfig unchanged.")
return
Expand Down
Loading