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

[release-4.15] OCPBUGS-31274: IBMCloud: Restrict CIS and DNS Service lookup #8197

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
10 changes: 7 additions & 3 deletions pkg/asset/installconfig/ibmcloud/metadata.go
Expand Up @@ -27,6 +27,7 @@ type Metadata struct {
computeSubnets map[string]Subnet
controlPlaneSubnets map[string]Subnet
dnsInstance *DNSInstance
publishStrategy types.PublishingStrategy
serviceEndpoints []configv1.IBMCloudServiceEndpoint

mutex sync.Mutex
Expand All @@ -46,6 +47,7 @@ func NewMetadata(config *types.InstallConfig) *Metadata {
BaseDomain: config.BaseDomain,
ComputeSubnetNames: config.Platform.IBMCloud.ComputeSubnets,
ControlPlaneSubnetNames: config.Platform.IBMCloud.ControlPlaneSubnets,
publishStrategy: config.Publish,
Region: config.Platform.IBMCloud.Region,
serviceEndpoints: config.Platform.IBMCloud.ServiceEndpoints,
}
Expand Down Expand Up @@ -79,7 +81,8 @@ func (m *Metadata) CISInstanceCRN(ctx context.Context) (string, error) {
m.mutex.Lock()
defer m.mutex.Unlock()

if m.cisInstanceCRN == "" {
// Only attempt to find the CIS instance if using ExternalPublishingStrategy and we have not collected it already
if m.publishStrategy == types.ExternalPublishingStrategy && m.cisInstanceCRN == "" {
client, err := m.Client()
if err != nil {
return "", err
Expand Down Expand Up @@ -111,8 +114,9 @@ func (m *Metadata) DNSInstance(ctx context.Context) (*DNSInstance, error) {
m.mutex.Lock()
defer m.mutex.Unlock()

// Prevent multiple attempts to retrieve (set) the dnsInstance if it hasn't been set (multiple threads reach mutex concurrently)
if m.dnsInstance == nil {
// Only attempt to find the DNS Services instance if using InternalPublishingStrategy and also
// prevent multiple attempts to retrieve (set) the dnsInstance if it hasn't been set (multiple threads reach mutex concurrently)
if m.publishStrategy == types.InternalPublishingStrategy && m.dnsInstance == nil {
client, err := m.Client()
if err != nil {
return nil, err
Expand Down
7 changes: 7 additions & 0 deletions pkg/asset/installconfig/ibmcloud/metadata_test.go
Expand Up @@ -178,9 +178,14 @@ func baseMetadata() *Metadata {
Region: region,
},
},
Publish: types.ExternalPublishingStrategy,
})
}

func setInternalPublishingStrategy(m *Metadata) {
m.publishStrategy = types.InternalPublishingStrategy
}

func TestAccountID(t *testing.T) {
testCases := []struct {
name string
Expand Down Expand Up @@ -406,6 +411,7 @@ func TestDNSInstance(t *testing.T) {
for _, tCase := range testCases {
t.Run(tCase.name, func(t *testing.T) {
metadata := baseMetadata()
setInternalPublishingStrategy(metadata)
metadata.client = ibmcloudClient
for _, edit := range tCase.edits {
edit(metadata)
Expand Down Expand Up @@ -438,6 +444,7 @@ func TestSetDNSInstance(t *testing.T) {
for _, tCase := range testCases {
t.Run(tCase.name, func(t *testing.T) {
metadata := baseMetadata()
setInternalPublishingStrategy(metadata)

metadata.dnsInstance = &DNSInstance{
ID: tCase.dnsID,
Expand Down