Skip to content

Commit

Permalink
Use a per-hcp router for all hcp ingress traffic (openshift#1614)
Browse files Browse the repository at this point in the history 
* Expose through router

* KAS service reporting: Report on router service if exposed through route

* Revert "KAS service reporting: Report on router service if exposed through route"

This reverts commit 219b149.

* Default KAS port to 443 when exposing through router

This entails having to listen on a different port than what is used
externally, as the KAS doesn't run as route and thus can not bind to a
port < 1025.

* Create router earlier so lb services get provisioned asap

* Revert "Revert "KAS service reporting: Report on router service if exposed through route""

This reverts commit 9f52874.

* Rebase

* Be a bit more explicit
  • Loading branch information
@alvaroaleman
alvaroaleman authored Aug 3, 2022
1 parent aadcf32 commit 0b95b70
Show file tree
Hide file tree
Showing 33 changed files with 2,007 additions and 253 deletions.
20 changes: 12 additions & 8 deletions api/fixtures/example.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,13 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token
},
}
}
services = getIngressServicePublishingStrategyMapping(o.NetworkType)
services = getIngressServicePublishingStrategyMapping(o.NetworkType, o.ExternalDNSDomain != "")
if o.ExternalDNSDomain != "" {
for i, svc := range services {
switch svc.Service {
case hyperv1.APIServer:
if endpointAccess != hyperv1.Private {
services[i].LoadBalancer = &hyperv1.LoadBalancerPublishingStrategy{
services[i].Route = &hyperv1.RoutePublishingStrategy{
Hostname: fmt.Sprintf("api-%s.%s", o.Name, o.ExternalDNSDomain),
}
}
Expand Down Expand Up @@ -262,7 +262,7 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token
if o.None.APIServerAddress != "" {
services = getServicePublishingStrategyMappingByAPIServerAddress(o.None.APIServerAddress, o.NetworkType)
} else {
services = getIngressServicePublishingStrategyMapping(o.NetworkType)
services = getIngressServicePublishingStrategyMapping(o.NetworkType, o.ExternalDNSDomain != "")
}
case o.Agent != nil:
platformSpec = hyperv1.PlatformSpec{
Expand Down Expand Up @@ -300,7 +300,7 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token
case "NodePort":
services = getServicePublishingStrategyMappingByAPIServerAddress(o.Kubevirt.APIServerAddress, o.NetworkType)
case "Ingress":
services = getIngressServicePublishingStrategyMapping(o.NetworkType)
services = getIngressServicePublishingStrategyMapping(o.NetworkType, o.ExternalDNSDomain != "")
default:
panic(fmt.Sprintf("service publishing type %s is not supported", o.Kubevirt.ServicePublishingStrategy))
}
Expand Down Expand Up @@ -337,7 +337,7 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token
SecurityGroupName: o.Azure.SecurityGroupName,
},
}
services = getIngressServicePublishingStrategyMapping(o.NetworkType)
services = getIngressServicePublishingStrategyMapping(o.NetworkType, o.ExternalDNSDomain != "")

case o.PowerVS != nil:
buildIBMCloudCreds := func(name, apikey string) *corev1.Secret {
Expand Down Expand Up @@ -394,7 +394,7 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token
IngressOperatorCloudCreds: corev1.LocalObjectReference{Name: powerVSResources.IngressOperatorCloudCreds.Name},
},
}
services = getIngressServicePublishingStrategyMapping(o.NetworkType)
services = getIngressServicePublishingStrategyMapping(o.NetworkType, o.ExternalDNSDomain != "")
default:
panic("no platform specified")
}
Expand Down Expand Up @@ -615,13 +615,17 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token
}
}

func getIngressServicePublishingStrategyMapping(netType hyperv1.NetworkType) []hyperv1.ServicePublishingStrategyMapping {
func getIngressServicePublishingStrategyMapping(netType hyperv1.NetworkType, usesExternalDNS bool) []hyperv1.ServicePublishingStrategyMapping {

apiServiceStrategy := hyperv1.LoadBalancer
if usesExternalDNS {
apiServiceStrategy = hyperv1.Route
}
ret := []hyperv1.ServicePublishingStrategyMapping{
{
Service: hyperv1.APIServer,
ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{
Type: hyperv1.LoadBalancer,
Type: apiServiceStrategy,
},
},
{
Expand Down
6 changes: 5 additions & 1 deletion api/v1alpha1/endpointservice_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,14 @@ type AWSEndpointServiceStatus struct {
// +optional
EndpointID string `json:"endpointID,omitempty"`

// DNSName is the name for the record created in the hypershift private zone
// Deprecated: Use DNSNames instead
// +optional
DNSName string `json:"dnsName,omitempty"`

// DNSName are the names for the records created in the hypershift private zone
// +optional
DNSNames []string `json:"dnsNames,omitempty"`

// DNSZoneID is ID for the hypershift private zone
// +optional
DNSZoneID string `json:"dnsZoneID,omitempty"`
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions cmd/install/assets/hypershift-operator/hypershift.openshift.io_awsendpointservices.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,14 @@ spec:
type: object
type: array
dnsName:
description: DNSName is the name for the record created in the hypershift
private zone
description: 'Deprecated: Use DNSNames instead'
type: string
dnsNames:
description: DNSName are the names for the records created in the
hypershift private zone
items:
type: string
type: array
dnsZoneID:
description: DNSZoneID is ID for the hypershift private zone
type: string
Expand Down
81 changes: 56 additions & 25 deletions control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,12 +473,8 @@ func reconcileAWSEndpointService(ctx context.Context, awsEndpointService *hyperv
return nil
}

var recordName string
if awsEndpointService.Name == manifests.KubeAPIServerPrivateService("").Name {
recordName = "api"
} else if awsEndpointService.Name == manifests.PrivateRouterService("").Name {
recordName = "*.apps"
} else {
recordNames := recordsForService(awsEndpointService, hcp)
if len(recordNames) == 0 {
log.Info("WARNING: no mapping from AWSEndpointService to DNS")
return nil
}
Expand All @@ -489,19 +485,45 @@ func reconcileAWSEndpointService(ctx context.Context, awsEndpointService *hyperv
return err
}

fqdn := fmt.Sprintf("%s.%s", recordName, zoneName)
err = createRecord(ctx, route53Client, zoneID, fqdn, *(endpointDNSEntries[0].DnsName))
if err != nil {
return err
var fqdns []string
for _, recordName := range recordNames {
fqdn := fmt.Sprintf("%s.%s", recordName, zoneName)
fqdns = append(fqdns, fqdn)
err = createRecord(ctx, route53Client, zoneID, fqdn, *(endpointDNSEntries[0].DnsName))
if err != nil {
return err
}
log.Info("DNS record created", "fqdn", fqdn)
}
log.Info("DNS record created", "fqdn", fqdn)

awsEndpointService.Status.DNSName = fqdn
//lint:ignore SA1019 we reset the deprecated field precicely
// because it is deprecated.
awsEndpointService.Status.DNSName = ""
awsEndpointService.Status.DNSNames = fqdns
awsEndpointService.Status.DNSZoneID = zoneID

return nil
}

func recordsForService(awsEndpointService *hyperv1.AWSEndpointService, hcp *hyperv1.HostedControlPlane) []string {
if awsEndpointService.Name == manifests.KubeAPIServerPrivateService("").Name {
return []string{"api"}

}
if awsEndpointService.Name != manifests.PrivateRouterService("").Name {
return nil
}

// If the kas is exposed through a route, the router needs to have DNS entries for both
// the kas and the apps domain
if m := servicePublishingStrategyByType(hcp, hyperv1.APIServer); m != nil && m.Type == hyperv1.Route {
return []string{"api", "*.apps"}
}

return []string{"*.apps"}

}

func apiTagToEC2Tag(name string, in []hyperv1.AWSResourceTag) []*ec2.Tag {
result := make([]*ec2.Tag, len(in))
for _, val := range in {
Expand Down Expand Up @@ -538,28 +560,37 @@ func (r *AWSEndpointServiceReconciler) delete(ctx context.Context, awsEndpointSe
log.Info("endpoint deleted", "endpointID", endpointID)
}

fqdn := awsEndpointService.Status.DNSName
zoneID := awsEndpointService.Status.DNSZoneID
if err != nil {
return false, err
}
if fqdn != "" && zoneID != "" {
record, err := findRecord(ctx, route53Client, zoneID, fqdn)
if err != nil {
return false, err
}
if record != nil {
err = deleteRecord(ctx, route53Client, zoneID, record)

for _, fqdn := range awsEndpointService.Status.DNSNames {
if fqdn != "" && zoneID != "" {
record, err := findRecord(ctx, route53Client, zoneID, fqdn)
if err != nil {
return false, err
}
log.Info("DNS record deleted", "fqdn", fqdn)
} else {
log.Info("no DNS record found", "fqdn", fqdn)
if record != nil {
err = deleteRecord(ctx, route53Client, zoneID, record)
if err != nil {
return false, err
}
log.Info("DNS record deleted", "fqdn", fqdn)
} else {
log.Info("no DNS record found", "fqdn", fqdn)
}
}
} else {
log.Info("no DNS status set in AWSEndpointService", "name", awsEndpointService.Name)
}

return true, nil
}

func servicePublishingStrategyByType(hcp *hyperv1.HostedControlPlane, svcType hyperv1.ServiceType) *hyperv1.ServicePublishingStrategy {
for _, mapping := range hcp.Spec.Services {
if mapping.Service == svcType {
return &mapping.ServicePublishingStrategy
}
}
return nil
}
47 changes: 47 additions & 0 deletions control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ package awsprivatelink
import (
"reflect"
"testing"

"github.com/google/go-cmp/cmp"
hyperv1 "github.com/openshift/hypershift/api/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func Test_diffSubnetIDs(t *testing.T) {
Expand Down Expand Up @@ -77,3 +81,46 @@ func Test_diffSubnetIDs(t *testing.T) {
})
}
}

func TestRecordForService(t *testing.T) {
testCases := []struct {
name string
in *hyperv1.AWSEndpointService
serviceMapping []hyperv1.ServicePublishingStrategyMapping
expected []string
}{
{
name: "Unknown service, no entry",
in: &hyperv1.AWSEndpointService{ObjectMeta: metav1.ObjectMeta{Name: "unknown"}},
},
{
name: "KAS service gets api entry",
in: &hyperv1.AWSEndpointService{ObjectMeta: metav1.ObjectMeta{Name: "kube-apiserver-private"}},
expected: []string{"api"},
},
{
name: "Router service gets api and apps entry when kas is exposed through route",
in: &hyperv1.AWSEndpointService{ObjectMeta: metav1.ObjectMeta{Name: "private-router"}},
serviceMapping: []hyperv1.ServicePublishingStrategyMapping{{
Service: hyperv1.APIServer,
ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{Type: hyperv1.Route},
}},
expected: []string{"api", "*.apps"},
},
{
name: "Router service gets apps entry only when kas is not exposed through route",
in: &hyperv1.AWSEndpointService{ObjectMeta: metav1.ObjectMeta{Name: "private-router"}},
expected: []string{"*.apps"},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
hcp := &hyperv1.HostedControlPlane{Spec: hyperv1.HostedControlPlaneSpec{Services: tc.serviceMapping}}
actual := recordsForService(tc.in, hcp)
if diff := cmp.Diff(actual, tc.expected); diff != "" {
t.Errorf("actual differs from expected: %s", diff)
}
})
}
}
2 changes: 1 addition & 1 deletion control-plane-operator/controllers/hostedcontrolplane/cno/clusternetworkoperator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func NewParams(hcp *hyperv1.HostedControlPlane, version string, images map[strin
p.DeploymentConfig.SetDefaultSecurityContext = setDefaultSecurityContext
if util.IsPrivateHCP(hcp) {
p.APIServerAddress = fmt.Sprintf("api.%s.hypershift.local", hcp.Name)
p.APIServerPort = 6443
p.APIServerPort = util.APIPortWithDefault(hcp, config.DefaultAPIServerPort)
} else {
p.APIServerAddress = hcp.Status.ControlPlaneEndpoint.Host
p.APIServerPort = hcp.Status.ControlPlaneEndpoint.Port
Expand Down
Loading

0 comments on commit 0b95b70

Please sign in to comment.