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

SDK2: Ensure service endpoints track2 #3885

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
9 changes: 8 additions & 1 deletion pkg/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,12 @@ type manager struct {
fpPrivateEndpoints network.PrivateEndpointsClient // TODO: use armFPPrivateEndpoints instead.
armFPPrivateEndpoints armnetwork.PrivateEndpointsClient
armRPPrivateLinkServices armnetwork.PrivateLinkServicesClient
armSubnets armnetwork.SubnetsClient
userAssignedIdentities armmsi.UserAssignedIdentitiesClient

dns dns.Manager
storage storage.Manager
subnet subnet.Manager
subnet subnet.Manager // TODO: use armSubnet instead.

Choose a reason for hiding this comment

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

TODO should be removed or carded up?

graph graph.Manager
rpBlob azblob.Manager

Expand Down Expand Up @@ -225,6 +226,11 @@ func New(ctx context.Context, log *logrus.Entry, _env env.Interface, db database
return nil, err
}

armSubnetsClient, err := armnetwork.NewSubnetsClient(r.SubscriptionID, fpCredClusterTenant, clientOptions)
if err != nil {
return nil, err
}

armRoleDefinitionsClient, err := armauthorization.NewArmRoleDefinitionsClient(fpCredClusterTenant, clientOptions)
if err != nil {
return nil, err
Expand Down Expand Up @@ -266,6 +272,7 @@ func New(ctx context.Context, log *logrus.Entry, _env env.Interface, db database
fpPrivateEndpoints: network.NewPrivateEndpointsClient(_env.Environment(), _env.SubscriptionID(), localFPAuthorizer),
armFPPrivateEndpoints: armFPPrivateEndpoints,
armRPPrivateLinkServices: armRPPrivateLinkServices,
armSubnets: armSubnetsClient,

dns: dns.NewManager(_env, fpCredRPTenant),
storage: storage,
Expand Down
76 changes: 75 additions & 1 deletion pkg/cluster/ensureendpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ package cluster
import (
"context"
"fmt"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v2"
"github.com/Azure/go-autorest/autorest/to"

"github.com/Azure/ARO-RP/pkg/api"
)
Expand All @@ -14,12 +19,35 @@ import (
// subnets for storage account access, but only if egress lockdown is
// not enabled.
func (m *manager) ensureServiceEndpoints(ctx context.Context) error {
// Only add service endpoints to the subnet if egress lockdown is not enabled.
if m.doc.OpenShiftCluster.Properties.FeatureProfile.GatewayEnabled {
return nil
}

subnetIds, err := m.getSubnetIds()
if err != nil {
return err
}

return m.subnet.CreateOrUpdateFromIds(ctx, subnetIds, m.doc.OpenShiftCluster.Properties.FeatureProfile.GatewayEnabled)
for _, subnetId := range subnetIds {
r, err := arm.ParseResourceID(subnetId)
if err != nil {
return err
}
subnet, err := m.armSubnets.Get(ctx, r.ResourceGroupName, r.Parent.Name, r.Name, nil)
if err != nil {
return err
}
shouldUpdate := addEndpointsToSubnet(api.SubnetsEndpoints, &subnet.Subnet)
if !shouldUpdate {
continue
}
err = m.armSubnets.CreateOrUpdateAndWait(ctx, r.ResourceGroupName, r.Parent.Name, r.Name, subnet.Subnet, nil)
if err != nil {
return err
}
}
return nil
}

func (m *manager) getSubnetIds() ([]string, error) {
Expand All @@ -36,3 +64,49 @@ func (m *manager) getSubnetIds() ([]string, error) {
}
return subnets, nil
}

// addEndpointsToSubnet adds the endpoints (that either are missing in subnet
// or aren't in succeeded state in the subnet) to the subnet and returns the updated subnet
func addEndpointsToSubnet(endpoints []string, subnet *armnetwork.Subnet) (subnetChanged bool) {
for _, endpoint := range endpoints {
endpointFound, serviceEndpointPtr := subnetContainsEndpoint(subnet, endpoint)

if !endpointFound || *serviceEndpointPtr.ProvisioningState != armnetwork.ProvisioningStateSucceeded {

Choose a reason for hiding this comment

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

Can ProvisioningState ever be in an error state? We wouldn't want to add it then I'm guessing

addEndpointToSubnet(endpoint, subnet)
subnetChanged = true
}
}

return subnetChanged
}

// subnetContainsEndpoint returns false and nil if subnet does not contain the endpoint.
// If the subnet does contain the endpoint, true and a pointer to the service endpoint
// is returned to be able to do additional checks and perform actions accordingly.
func subnetContainsEndpoint(subnet *armnetwork.Subnet, endpoint string) (endpointFound bool, serviceEndpointPtr *armnetwork.ServiceEndpointPropertiesFormat) {
if subnet == nil || subnet.Properties.ServiceEndpoints == nil {
return false, nil
}

for _, serviceEndpoint := range subnet.Properties.ServiceEndpoints {
if endpointFound = strings.EqualFold(*serviceEndpoint.Service, endpoint); endpointFound {
return true, serviceEndpoint
}
}

return false, nil
}

// addEndpointToSubnet appends the endpoint to the slice of ServiceEndpoints of the subnet.
func addEndpointToSubnet(endpoint string, subnet *armnetwork.Subnet) {
if subnet.Properties.ServiceEndpoints == nil {
subnet.Properties.ServiceEndpoints = []*armnetwork.ServiceEndpointPropertiesFormat{}
}

serviceEndpoint := armnetwork.ServiceEndpointPropertiesFormat{
Service: to.StringPtr(endpoint),
Locations: []*string{to.StringPtr("*")},
}

subnet.Properties.ServiceEndpoints = append(subnet.Properties.ServiceEndpoints, &serviceEndpoint)
}
Loading
Loading