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

Allow ssl and ip address whitelisting fields to be updated on configure #2305

Merged
merged 3 commits into from
Jul 2, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cli/cmd/lib_cluster_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,10 @@ func confirmInstallClusterConfig(clusterConfig *clusterconfig.Config, awsClient
func confirmConfigureClusterConfig(configureChanges clusterconfig.ConfigureChanges, oldCc, newCc clusterconfig.Config, disallowPrompt bool) {
fmt.Printf("your %s cluster in region %s will be updated as follows:\n\n", newCc.ClusterName, newCc.Region)

for _, fieldToUpdate := range configureChanges.FieldsToUpdate {
fmt.Printf("○ %s will be updated\n", fieldToUpdate)
}

for _, ngName := range configureChanges.NodeGroupsToScale {
ngOld := oldCc.GetNodeGroupByName(ngName)
ngScaled := newCc.GetNodeGroupByName(ngName)
Expand Down
47 changes: 47 additions & 0 deletions manager/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ function cluster_configure() {
add_nodegroups
remove_nodegroups

update_networking

echo -n "○ updating cluster configuration "
setup_configmap
echo "✓"
Expand Down Expand Up @@ -365,6 +367,51 @@ function setup_istio() {
output_if_error istio-${ISTIO_VERSION}/bin/istioctl install --skip-confirmation --filename /workspace/istio.yaml
}

function update_networking() {
prev_ssl_certificate_arn=$(kubectl get svc ingressgateway-apis -n=istio-system -o json | jq -r '.metadata.annotations."service.beta.kubernetes.io/aws-load-balancer-ssl-cert"')

if [ "$prev_ssl_certificate_arn" = "null" ]; then
prev_ssl_certificate_arn=""
fi

new_ssl_certificate_arn=$(cat $CORTEX_CLUSTER_CONFIG_FILE | yq -r .ssl_certificate_arn)

if [ "$new_ssl_certificate_arn" = "null" ]; then
new_ssl_certificate_arn=""
fi

prev_api_whitelist_ip_address=$(kubectl get svc ingressgateway-apis -n=istio-system -o yaml | yq -r -c ".spec.loadBalancerSourceRanges")
prev_operator_whitelist_ip_address=$(kubectl get svc ingressgateway-operator -n=istio-system -o yaml | yq -r -c ".spec.loadBalancerSourceRanges")

new_api_whitelist_ip_address=$(cat $CORTEX_CLUSTER_CONFIG_FILE | yq -r -c ".api_load_balancer_cidr_white_list")
new_operator_whitelist_ip_address=$(cat $CORTEX_CLUSTER_CONFIG_FILE | yq -r -c ".operator_load_balancer_cidr_white_list")

if [ "$prev_ssl_certificate_arn" = "$new_ssl_certificate_arn" ] && [ "$prev_api_whitelist_ip_address" = "$new_api_whitelist_ip_address" ] && [ "$prev_operator_whitelist_ip_address" = "$new_operator_whitelist_ip_address" ] ; then
return
fi

echo -n "○ updating networking configuration "

if [ "$new_ssl_certificate_arn" != "$prev_ssl_certificate_arn" ] ; then
# there is a bug where changing the certificate annotation will not cause the HTTPS listener in the NLB to update
# the current workaround is to delete the HTTPS listener and have it recreated with istioctl
if [ "$prev_ssl_certificate_arn" != "" ] ; then
kubectl patch svc ingressgateway-apis -n=istio-system --type=json -p="[{'op': 'remove', 'path': '/metadata/annotations/service.beta.kubernetes.io~1aws-load-balancer-ssl-cert'}]" >/dev/null
fi
https_index=$(kubectl get svc ingressgateway-apis -n=istio-system -o json | jq '.spec.ports | map(.name == "https") | index(true)')
if [ "$https_index" != "null" ] ; then
kubectl patch svc ingressgateway-apis -n=istio-system --type=json -p="[{'op': 'remove', 'path': '/spec/ports/$https_index'}]" >/dev/null
fi
fi

python render_template.py $CORTEX_CLUSTER_CONFIG_FILE manifests/istio.yaml.j2 > /workspace/istio.yaml
output_if_error istio-${ISTIO_VERSION}/bin/istioctl install --skip-confirmation --filename /workspace/istio.yaml
python render_template.py $CORTEX_CLUSTER_CONFIG_FILE manifests/apis.yaml.j2 > /workspace/apis.yaml
kubectl apply -f /workspace/apis.yaml >/dev/null

echo "✓"
}

function validate_cortex() {
set +e

Expand Down
8 changes: 2 additions & 6 deletions manager/manifests/istio.yaml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ spec:
service:
type: LoadBalancer
externalTrafficPolicy: Cluster # https://medium.com/pablo-perez/k8s-externaltrafficpolicy-local-or-cluster-40b259a19404, https://www.asykim.com/blog/deep-dive-into-kubernetes-external-traffic-policies
{% if config.get('operator_load_balancer_cidr_white_list', [])|length > 0 %}
loadBalancerSourceRanges: {{ config['operator_load_balancer_cidr_white_list'] }}
{% endif %}
loadBalancerSourceRanges: {{ config.get('operator_load_balancer_cidr_white_list', ['0.0.0.0/0']) }}
selector:
app: operator-istio-gateway
istio: ingressgateway-operator
Expand Down Expand Up @@ -110,9 +108,7 @@ spec:
{% endif %}
service:
type: LoadBalancer
{% if config.get('api_load_balancer_cidr_white_list', [])|length > 0 %}
loadBalancerSourceRanges: {{ config['api_load_balancer_cidr_white_list'] }}
{% endif %}
loadBalancerSourceRanges: {{ config.get('api_load_balancer_cidr_white_list', ['0.0.0.0/0']) }}
externalTrafficPolicy: Cluster # https://medium.com/pablo-perez/k8s-externaltrafficpolicy-local-or-cluster-40b259a19404, https://www.asykim.com/blog/deep-dive-into-kubernetes-external-traffic-policies
selector:
app: apis-istio-gateway
Expand Down
59 changes: 39 additions & 20 deletions pkg/types/clusterconfig/cluster_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/cortexlabs/cortex/pkg/lib/pointer"
"github.com/cortexlabs/cortex/pkg/lib/sets/strset"
"github.com/cortexlabs/cortex/pkg/lib/slices"
libstr "github.com/cortexlabs/cortex/pkg/lib/strings"
"github.com/cortexlabs/cortex/pkg/lib/structs"
"github.com/cortexlabs/yaml"
)
Expand Down Expand Up @@ -191,10 +192,11 @@ type ConfigureChanges struct {
NodeGroupsToRemove []string
NodeGroupsToScale []string
EKSNodeGroupsToRemove []string // EKS node group names of (NodeGroupsToRemove ∩ Cortex-converted EKS node groups) ∪ (Cortex-converted EKS node groups - the new cluster config's nodegroups)
FieldsToUpdate []string
}

func (c *ConfigureChanges) HasChanges() bool {
return len(c.NodeGroupsToAdd) != 0 || len(c.NodeGroupsToRemove) != 0 || len(c.NodeGroupsToScale) != 0 || len(c.EKSNodeGroupsToRemove) != 0
return len(c.NodeGroupsToAdd)+len(c.NodeGroupsToRemove)+len(c.NodeGroupsToScale)+len(c.EKSNodeGroupsToRemove)+len(c.FieldsToUpdate) != 0
}

// GetGhostEKSNodeGroups returns the set difference between EKSNodeGroupsToRemove and the EKS-converted NodeGroupsToRemove
Expand Down Expand Up @@ -1029,43 +1031,54 @@ func (cc *Config) validate(awsClient *aws.Client) error {
return nil
}

func (cc *Config) validateConfigDiff(oldConfig Config) error {
err := cc.validateTopLevelSectionDiff(oldConfig)
if err != nil {
return err
}

return cc.validateSharedNodeGroupsDiff(oldConfig)
}

func (cc *Config) validateTopLevelSectionDiff(oldConfig Config) error {
func (cc *Config) validateTopLevelSectionDiff(oldConfig Config) ([]string, error) {
var fieldsToUpdate []string
// validate actionable changes
newClusterConfigCopy, err := cc.DeepCopy()
if err != nil {
return err
return nil, err
}

oldClusterConfigCopy, err := oldConfig.DeepCopy()
if err != nil {
return err
return nil, err
}

if libstr.Obj(newClusterConfigCopy.SSLCertificateARN) != libstr.Obj(oldClusterConfigCopy.SSLCertificateARN) {
fieldsToUpdate = append(fieldsToUpdate, SSLCertificateARNKey)
}

newClusterConfigCopy.NodeGroups = []*NodeGroup{}
oldClusterConfigCopy.NodeGroups = []*NodeGroup{}
if libstr.Obj(newClusterConfigCopy.APILoadBalancerCIDRWhiteList) != libstr.Obj(oldClusterConfigCopy.APILoadBalancerCIDRWhiteList) {
fieldsToUpdate = append(fieldsToUpdate, APILoadBalancerCIDRWhiteListKey)
}

if libstr.Obj(newClusterConfigCopy.OperatorLoadBalancerCIDRWhiteList) != libstr.Obj(oldClusterConfigCopy.OperatorLoadBalancerCIDRWhiteList) {
fieldsToUpdate = append(fieldsToUpdate, OperatorLoadBalancerCIDRWhiteListKey)
}

clearUpdatableFields(&newClusterConfigCopy)
clearUpdatableFields(&oldClusterConfigCopy)

h1, err := newClusterConfigCopy.Hash()
if err != nil {
return err
return nil, err
}
h2, err := oldClusterConfigCopy.Hash()
if err != nil {
return err
return nil, err
}
if h1 != h2 {
return ErrorConfigCannotBeChangedOnConfigure()
return nil, ErrorConfigCannotBeChangedOnConfigure()
}

return nil
return fieldsToUpdate, nil
}

func clearUpdatableFields(clusterConfig *Config) {
clusterConfig.SSLCertificateARN = nil
clusterConfig.APILoadBalancerCIDRWhiteList = nil
clusterConfig.OperatorLoadBalancerCIDRWhiteList = nil
clusterConfig.NodeGroups = []*NodeGroup{}
}

func (cc *Config) validateSharedNodeGroupsDiff(oldConfig Config) error {
Expand Down Expand Up @@ -1139,7 +1152,12 @@ func (cc *Config) ValidateOnConfigure(awsClient *aws.Client, oldConfig Config, e
return ConfigureChanges{}, err
}

err = cc.validateConfigDiff(oldConfig)
fieldsToUpdate, err := cc.validateTopLevelSectionDiff(oldConfig)
if err != nil {
return ConfigureChanges{}, err
}

err = cc.validateSharedNodeGroupsDiff(oldConfig)
if err != nil {
return ConfigureChanges{}, err
}
Expand Down Expand Up @@ -1170,6 +1188,7 @@ func (cc *Config) ValidateOnConfigure(awsClient *aws.Client, oldConfig Config, e
NodeGroupsToRemove: GetNodeGroupNames(ngsToBeRemoved),
NodeGroupsToScale: GetNodeGroupNames(ngNamesToBeScaled),
EKSNodeGroupsToRemove: getStaleEksNodeGroups(cc.ClusterName, eksNodeGroupStacks, cc.NodeGroups, ngsToBeRemoved),
FieldsToUpdate: fieldsToUpdate,
}, nil
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/types/clusterconfig/config_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ const (
NATGatewayKey = "nat_gateway"
APILoadBalancerSchemeKey = "api_load_balancer_scheme"
OperatorLoadBalancerSchemeKey = "operator_load_balancer_scheme"
APILoadBalancerCIDRWhiteListKey = "api_load_balancer_cidr_white_list"
OperatorLoadBalancerCIDRWhiteListKey = "operator_load_balancer_cidr_white_list"
VPCCIDRKey = "vpc_cidr"
AccountIDKey = "account_id"
TelemetryKey = "telemetry"
Expand Down
2 changes: 1 addition & 1 deletion pkg/types/clusterconfig/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ func ErrorNoNATGatewayWithSubnets() error {
func ErrorConfigCannotBeChangedOnConfigure() error {
return errors.WithStack(&errors.Error{
Kind: ErrConfigCannotBeChangedOnConfigure,
Message: fmt.Sprintf("in a running cluster, only the %s field can be modified", NodeGroupsKey),
Message: fmt.Sprintf("in a running cluster, only %s can be modified", s.StrsAnd([]string{NodeGroupsKey, SSLCertificateARNKey, OperatorLoadBalancerCIDRWhiteListKey, APILoadBalancerCIDRWhiteListKey})),
})
}

Expand Down