Skip to content

Commit

Permalink
code polish.
Browse files Browse the repository at this point in the history
Signed-off-by: qicz <qiczzhu@gmail.com>
  • Loading branch information
qicz committed Mar 1, 2023
1 parent a23cd3e commit 54a2ea0
Show file tree
Hide file tree
Showing 33 changed files with 364 additions and 402 deletions.
2 changes: 1 addition & 1 deletion internal/cmd/egctl/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ func portForwarder(nn types.NamespacedName) (kube.PortForwarder, error) {
return nil, fmt.Errorf("pod %s is not running", nn)
}

fw, err := kube.NewLocalPortForwarder(c, nn, 0, int(adminPort))
fw, err := kube.NewLocalPortForwarder(c, nn, 0, adminPort)
if err != nil {
return nil, err
}
Expand Down
8 changes: 4 additions & 4 deletions internal/cmd/egctl/translate.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (

adminv3 "github.com/envoyproxy/go-control-plane/envoy/admin/v3"
bootstrapv3 "github.com/envoyproxy/go-control-plane/envoy/config/bootstrap/v3"
resource "github.com/envoyproxy/go-control-plane/pkg/resource/v3"
resourcev3 "github.com/envoyproxy/go-control-plane/pkg/resource/v3"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -289,7 +289,7 @@ func constructConfigDump(tCtx *xds_types.ResourceVersionTable) (*adminv3.ConfigD
}

// construct clusters config
clusters := tCtx.XdsResources[resource.ClusterType]
clusters := tCtx.XdsResources[resourcev3.ClusterType]
for _, cluster := range clusters {
c, err := anypb.New(cluster)
if err != nil {
Expand All @@ -307,7 +307,7 @@ func constructConfigDump(tCtx *xds_types.ResourceVersionTable) (*adminv3.ConfigD
}

// construct listeners config
listeners := tCtx.XdsResources[resource.ListenerType]
listeners := tCtx.XdsResources[resourcev3.ListenerType]
for _, listener := range listeners {
l, err := anypb.New(listener)
if err != nil {
Expand All @@ -327,7 +327,7 @@ func constructConfigDump(tCtx *xds_types.ResourceVersionTable) (*adminv3.ConfigD
}

// construct routes config
routes := tCtx.XdsResources[resource.RouteType]
routes := tCtx.XdsResources[resourcev3.RouteType]
for _, route := range routes {
r, err := anypb.New(route)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/envoygateway/config/decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func TestDecode(t *testing.T) {
require.NoError(t, err)
require.Equal(t, tc.out, eg)
} else {
require.Equal(t, (!reflect.DeepEqual(tc.out, eg) || err != nil), true)
require.Equal(t, !reflect.DeepEqual(tc.out, eg) || err != nil, true)
}
})
}
Expand Down
1 change: 0 additions & 1 deletion internal/gatewayapi/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ type ListenersTranslator interface {
}

func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR XdsIRMap, infraIR InfraIRMap, resources *Resources) {

t.validateConflictedLayer7Listeners(gateways)
t.validateConflictedLayer4Listeners(gateways, v1beta1.TCPProtocolType)
t.validateConflictedLayer4Listeners(gateways, v1beta1.UDPProtocolType)
Expand Down
4 changes: 2 additions & 2 deletions internal/gatewayapi/sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ func (x XdsIRRoutes) Less(i, j int) bool {
// defined in the Gateway API spec.
// https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1beta1.HTTPRouteRule
func sortXdsIRMap(xdsIR XdsIRMap) {
for _, ir := range xdsIR {
for _, http := range ir.HTTP {
for _, irItem := range xdsIR {
for _, http := range irItem.HTTP {
// descending order
sort.Sort(sort.Reverse(XdsIRRoutes(http.Routes)))
}
Expand Down
2 changes: 1 addition & 1 deletion internal/gatewayapi/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ func (t *Translator) validateTLSConfiguration(listener *ListenerContext, resourc
if certificateRef.Namespace != nil && string(*certificateRef.Namespace) != "" && string(*certificateRef.Namespace) != listener.gateway.Namespace {
if !t.validateCrossNamespaceRef(
crossNamespaceFrom{
group: string(v1beta1.GroupName),
group: v1beta1.GroupName,
kind: KindGateway,
namespace: listener.gateway.Namespace,
},
Expand Down
6 changes: 3 additions & 3 deletions internal/globalratelimit/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ func (r *Runner) translate(xdsIRs []*ir.Xds) (*ir.RateLimitInfra, error) {

for _, xdsIR := range xdsIRs {
for _, listener := range xdsIR.HTTP {
config := translator.BuildRateLimitServiceConfig(listener)
if config != nil {
str, err := translator.GetRateLimitServiceConfigStr(config)
cfg := translator.BuildRateLimitServiceConfig(listener)
if cfg != nil {
str, err := translator.GetRateLimitServiceConfigStr(cfg)
if err != nil {
return nil, fmt.Errorf("failed to get rate limit config string: %w", err)
}
Expand Down
9 changes: 9 additions & 0 deletions internal/infrastructure/kubernetes/infra.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@
package kubernetes

import (
"fmt"

"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/envoyproxy/gateway/internal/envoygateway/config"
"github.com/envoyproxy/gateway/internal/provider/utils"
)

// Infra manages the creation and deletion of Kubernetes infrastructure
Expand All @@ -27,3 +30,9 @@ func NewInfra(cli client.Client, cfg *config.Server) *Infra {
Namespace: cfg.Namespace,
}
}

// expectedResourceHashedName returns hashed resource name.
func expectedResourceHashedName(name string) string {
hashedName := utils.GetHashedName(name)
return fmt.Sprintf("%s-%s", config.EnvoyPrefix, hashedName)
}
11 changes: 2 additions & 9 deletions internal/infrastructure/kubernetes/proxy_configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,8 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/envoyproxy/gateway/internal/envoygateway/config"
"github.com/envoyproxy/gateway/internal/gatewayapi"
"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/provider/utils"
)

// expectedProxyConfigMap returns the expected ConfigMap based on the provided infra.
Expand All @@ -29,7 +27,7 @@ func (i *Infra) expectedProxyConfigMap(infra *ir.Infra) (*corev1.ConfigMap, erro
return &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: i.Namespace,
Name: expectedProxyConfigMapName(infra.Proxy.Name),
Name: expectedResourceHashedName(infra.Proxy.Name),
Labels: labels,
},
Data: map[string]string{
Expand All @@ -55,14 +53,9 @@ func (i *Infra) deleteProxyConfigMap(ctx context.Context, infra *ir.Infra) error
cm := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: i.Namespace,
Name: expectedProxyConfigMapName(infra.Proxy.Name),
Name: expectedResourceHashedName(infra.Proxy.Name),
},
}

return i.deleteConfigMap(ctx, cm)
}

func expectedProxyConfigMapName(proxyName string) string {
cMapName := utils.GetHashedName(proxyName)
return fmt.Sprintf("%s-%s", config.EnvoyPrefix, cMapName)
}
15 changes: 4 additions & 11 deletions internal/infrastructure/kubernetes/proxy_deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@ import (
"k8s.io/utils/pointer"

egcfgv1a1 "github.com/envoyproxy/gateway/api/config/v1alpha1"
"github.com/envoyproxy/gateway/internal/envoygateway/config"
"github.com/envoyproxy/gateway/internal/gatewayapi"
"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/provider/utils"
"github.com/envoyproxy/gateway/internal/xds/bootstrap"
)

Expand All @@ -35,11 +33,6 @@ const (
envoyHTTPSPort = int32(8443)
)

func expectedProxyDeploymentName(proxyName string) string {
deploymentName := utils.GetHashedName(proxyName)
return fmt.Sprintf("%s-%s", config.EnvoyPrefix, deploymentName)
}

// expectedProxyDeployment returns the expected Deployment based on the provided infra.
func (i *Infra) expectedProxyDeployment(infra *ir.Infra) (*appsv1.Deployment, error) {
containers, err := expectedProxyContainers(infra)
Expand Down Expand Up @@ -68,7 +61,7 @@ func (i *Infra) expectedProxyDeployment(infra *ir.Infra) (*appsv1.Deployment, er
},
ObjectMeta: metav1.ObjectMeta{
Namespace: i.Namespace,
Name: expectedProxyDeploymentName(infra.Proxy.Name),
Name: expectedResourceHashedName(infra.Proxy.Name),
Labels: labels,
},
Spec: appsv1.DeploymentSpec{
Expand All @@ -80,7 +73,7 @@ func (i *Infra) expectedProxyDeployment(infra *ir.Infra) (*appsv1.Deployment, er
},
Spec: corev1.PodSpec{
Containers: containers,
ServiceAccountName: expectedProxyServiceAccountName(infra.Proxy.Name),
ServiceAccountName: expectedResourceHashedName(infra.Proxy.Name),
AutomountServiceAccountToken: pointer.Bool(false),
TerminationGracePeriodSeconds: pointer.Int64(int64(300)),
DNSPolicy: corev1.DNSClusterFirst,
Expand All @@ -100,7 +93,7 @@ func (i *Infra) expectedProxyDeployment(infra *ir.Infra) (*appsv1.Deployment, er
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: expectedProxyConfigMapName(infra.Proxy.Name),
Name: expectedResourceHashedName(infra.Proxy.Name),
},
Items: []corev1.KeyToPath{
{
Expand Down Expand Up @@ -214,7 +207,7 @@ func (i *Infra) deleteProxyDeployment(ctx context.Context, infra *ir.Infra) erro
deploy := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: i.Namespace,
Name: expectedProxyDeploymentName(infra.Proxy.Name),
Name: expectedResourceHashedName(infra.Proxy.Name),
},
}

Expand Down
4 changes: 2 additions & 2 deletions internal/infrastructure/kubernetes/proxy_deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func TestExpectedProxyDeployment(t *testing.T) {
require.NoError(t, err)

// Check the deployment name is as expected.
assert.Equal(t, deploy.Name, expectedProxyDeploymentName(infra.Proxy.Name))
assert.Equal(t, deploy.Name, expectedResourceHashedName(infra.Proxy.Name))

// Check container details, i.e. env vars, labels, etc. for the deployment are as expected.
container := checkContainer(t, deploy, envoyContainerName, true)
Expand Down Expand Up @@ -226,7 +226,7 @@ func TestCreateOrUpdateProxyDeployment(t *testing.T) {
actual := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: kube.Namespace,
Name: expectedProxyDeploymentName(tc.in.Proxy.Name),
Name: expectedResourceHashedName(tc.in.Proxy.Name),
},
}
require.NoError(t, kube.Client.Get(context.Background(), client.ObjectKeyFromObject(actual), actual))
Expand Down
8 changes: 4 additions & 4 deletions internal/infrastructure/kubernetes/proxy_infra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,31 +76,31 @@ func TestCreateProxyInfra(t *testing.T) {
sa := &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Namespace: kube.Namespace,
Name: expectedProxyServiceAccountName(tc.in.Proxy.Name),
Name: expectedResourceHashedName(tc.in.Proxy.Name),
},
}
require.NoError(t, kube.Client.Get(context.Background(), client.ObjectKeyFromObject(sa), sa))

cm := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: kube.Namespace,
Name: expectedProxyConfigMapName(tc.in.Proxy.Name),
Name: expectedResourceHashedName(tc.in.Proxy.Name),
},
}
require.NoError(t, kube.Client.Get(context.Background(), client.ObjectKeyFromObject(cm), cm))

deploy := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: kube.Namespace,
Name: expectedProxyDeploymentName(tc.in.Proxy.Name),
Name: expectedResourceHashedName(tc.in.Proxy.Name),
},
}
require.NoError(t, kube.Client.Get(context.Background(), client.ObjectKeyFromObject(deploy), deploy))

svc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Namespace: kube.Namespace,
Name: expectedProxyServiceName(tc.in.Proxy.Name),
Name: expectedResourceHashedName(tc.in.Proxy.Name),
},
}
require.NoError(t, kube.Client.Get(context.Background(), client.ObjectKeyFromObject(svc), svc))
Expand Down
15 changes: 4 additions & 11 deletions internal/infrastructure/kubernetes/proxy_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,11 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"

"github.com/envoyproxy/gateway/internal/envoygateway/config"
"github.com/envoyproxy/gateway/internal/gatewayapi"
"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/provider/utils"
)

func expectedProxyServiceName(proxyName string) string {
svcName := utils.GetHashedName(proxyName)
return fmt.Sprintf("%s-%s", config.EnvoyPrefix, svcName)
}

// expectedproxyService returns the expected Service based on the provided infra.
// expectedProxyService returns the expected Service based on the provided infra.
func (i *Infra) expectedProxyService(infra *ir.Infra) (*corev1.Service, error) {
var ports []corev1.ServicePort
for _, listener := range infra.Proxy.Listeners {
Expand Down Expand Up @@ -53,7 +46,7 @@ func (i *Infra) expectedProxyService(infra *ir.Infra) (*corev1.Service, error) {
svc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Namespace: i.Namespace,
Name: expectedProxyServiceName(infra.Proxy.Name),
Name: expectedResourceHashedName(infra.Proxy.Name),
Labels: labels,
},
Spec: corev1.ServiceSpec{
Expand All @@ -69,7 +62,7 @@ func (i *Infra) expectedProxyService(infra *ir.Infra) (*corev1.Service, error) {
return svc, nil
}

// createOrUpdateproxyService creates a Service in the kube api server based on the provided infra,
// createOrUpdateProxyService creates a Service in the kube api server based on the provided infra,
// if it doesn't exist or updates it if it does.
func (i *Infra) createOrUpdateProxyService(ctx context.Context, infra *ir.Infra) error {
svc, err := i.expectedProxyService(infra)
Expand All @@ -84,7 +77,7 @@ func (i *Infra) deleteProxyService(ctx context.Context, infra *ir.Infra) error {
svc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Namespace: i.Namespace,
Name: expectedProxyServiceName(infra.Proxy.Name),
Name: expectedResourceHashedName(infra.Proxy.Name),
},
}

Expand Down
2 changes: 1 addition & 1 deletion internal/infrastructure/kubernetes/proxy_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func TestDesiredProxyService(t *testing.T) {
require.NoError(t, err)

// Check the service name is as expected.
assert.Equal(t, svc.Name, expectedProxyDeploymentName(infra.Proxy.Name))
assert.Equal(t, svc.Name, expectedResourceHashedName(infra.Proxy.Name))

checkServiceHasPort(t, svc, 80)
checkServiceHasPort(t, svc, 443)
Expand Down
11 changes: 2 additions & 9 deletions internal/infrastructure/kubernetes/proxy_serviceaccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,10 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/envoyproxy/gateway/internal/envoygateway/config"
"github.com/envoyproxy/gateway/internal/gatewayapi"
"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/provider/utils"
)

func expectedProxyServiceAccountName(proxyName string) string {
svcActName := utils.GetHashedName(proxyName)
return fmt.Sprintf("%s-%s", config.EnvoyPrefix, svcActName)
}

// expectedProxyServiceAccount returns the expected proxy serviceAccount.
func (i *Infra) expectedProxyServiceAccount(infra *ir.Infra) (*corev1.ServiceAccount, error) {
// Set the labels based on the owning gateway name.
Expand All @@ -38,7 +31,7 @@ func (i *Infra) expectedProxyServiceAccount(infra *ir.Infra) (*corev1.ServiceAcc
},
ObjectMeta: metav1.ObjectMeta{
Namespace: i.Namespace,
Name: expectedProxyServiceAccountName(infra.Proxy.Name),
Name: expectedResourceHashedName(infra.Proxy.Name),
Labels: labels,
},
}, nil
Expand All @@ -60,7 +53,7 @@ func (i *Infra) deleteProxyServiceAccount(ctx context.Context, infra *ir.Infra)
sa := &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Namespace: i.Namespace,
Name: expectedProxyServiceAccountName(infra.Proxy.Name),
Name: expectedResourceHashedName(infra.Proxy.Name),
},
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestExpectedProxyServiceAccount(t *testing.T) {
require.NoError(t, err)

// Check the serviceaccount name is as expected.
assert.Equal(t, sa.Name, expectedProxyServiceAccountName(infra.Proxy.Name))
assert.Equal(t, sa.Name, expectedResourceHashedName(infra.Proxy.Name))

wantLabels := envoyAppLabel()
wantLabels[gatewayapi.OwningGatewayNamespaceLabel] = "default"
Expand Down Expand Up @@ -199,7 +199,7 @@ func TestCreateOrUpdateProxyServiceAccount(t *testing.T) {
actual := &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Namespace: kube.Namespace,
Name: expectedProxyServiceAccountName(tc.in.Proxy.Name),
Name: expectedResourceHashedName(tc.in.Proxy.Name),
},
}
require.NoError(t, kube.Client.Get(context.Background(), client.ObjectKeyFromObject(actual), actual))
Expand Down
Loading

0 comments on commit 54a2ea0

Please sign in to comment.