github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-29 14:37:00 +00:00
Code Issues Projects Releases Wiki Activity

Validation

Browse Source
This commit is contained in:
bprashanth 2016-10-18 18:43:13 -07:00
parent 243bd5743b
commit 5029bb06e9
4 changed files with 121 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
pkg/api/validation/validation.go
View File

@ -2385,8 +2385,15 @@ var supportedSessionAffinityType = sets.NewString(string(api.ServiceAffinityClie
var supportedServiceType = sets.NewString(string(api.ServiceTypeClusterIP), string(api.ServiceTypeNodePort),
string(api.ServiceTypeLoadBalancer), string(api.ServiceTypeExternalName))
// ValidateService tests if required fields in the service are set.
// ValidateService tests if required fields/annotations of a Service are valid.
func ValidateService(service *api.Service) field.ErrorList {
allErrs := validateServiceFields(service)
allErrs = append(allErrs, validateServiceAnnotations(service, nil)...)
return allErrs
}
// validateServiceFields tests if required fields in the service are set.
func validateServiceFields(service *api.Service) field.ErrorList {
allErrs := ValidateObjectMeta(&service.ObjectMeta, true, ValidateServiceName, field.NewPath("metadata"))
specPath := field.NewPath("spec")
@ -2567,6 +2574,63 @@ func validateServicePort(sp *api.ServicePort, requireName, isHeadlessService boo
return allErrs
}
func validateServiceAnnotations(service *api.Service, oldService *api.Service) (allErrs field.ErrorList) {
// 2 annotations went from alpha to beta in 1.5: healthcheck-nodeport and
// external-traffic. The user cannot mix these. All updates to the alpha
// annotation are disallowed. The user must change both alpha annotations
// to beta before making any modifications, even though the system continues
// to respect the alpha version.
hcAlpha, healthCheckAlphaOk := service.Annotations[apiservice.AlphaAnnotationHealthCheckNodePort]
onlyLocalAlpha, onlyLocalAlphaOk := service.Annotations[apiservice.AlphaAnnotationExternalTraffic]
_, healthCheckBetaOk := service.Annotations[apiservice.BetaAnnotationHealthCheckNodePort]
_, onlyLocalBetaOk := service.Annotations[apiservice.BetaAnnotationExternalTraffic]
var oldHealthCheckAlpha, oldOnlyLocalAlpha string
var oldHealthCheckAlphaOk, oldOnlyLocalAlphaOk bool
if oldService != nil {
oldHealthCheckAlpha, oldHealthCheckAlphaOk = oldService.Annotations[apiservice.AlphaAnnotationHealthCheckNodePort]
oldOnlyLocalAlpha, oldOnlyLocalAlphaOk = oldService.Annotations[apiservice.AlphaAnnotationExternalTraffic]
}
hcValueChanged := oldHealthCheckAlphaOk && healthCheckAlphaOk && oldHealthCheckAlpha != hcAlpha
hcValueNew := !oldHealthCheckAlphaOk && healthCheckAlphaOk
hcValueGone := !healthCheckAlphaOk && !healthCheckBetaOk && oldHealthCheckAlphaOk
onlyLocalHCMismatch := onlyLocalBetaOk && healthCheckAlphaOk
// On upgrading to a 1.5 cluster, the user is locked in at the current
// alpha setting, till they modify the Service such that the pair of
// annotations are both beta. Basically this means we need to:
// Disallow updates to the alpha annotation.
// Disallow creating a Service with the alpha annotation.
// Disallow removing both alpha annotations. Removing the health-check
// annotation is rejected at a later stage anyway, so if we allow removing
// just onlyLocal we might leak the port.
// Disallow a single field from transitioning to beta. Mismatched annotations
// cause confusion.
// Ignore changes to the fields if they're both transitioning to beta.
// Allow modifications to Services in fields other than the alpha annotation.
if hcValueNew || hcValueChanged || hcValueGone || onlyLocalHCMismatch {
fieldPath := field.NewPath("metadata", "annotations").Key(apiservice.AlphaAnnotationHealthCheckNodePort)
msg := fmt.Sprintf("please replace the alpha annotation with the beta version %v",
apiservice.BetaAnnotationHealthCheckNodePort)
allErrs = append(allErrs, field.Invalid(fieldPath, apiservice.AlphaAnnotationHealthCheckNodePort, msg))
}
onlyLocalValueChanged := oldOnlyLocalAlphaOk && onlyLocalAlphaOk && oldOnlyLocalAlpha != onlyLocalAlpha
onlyLocalValueNew := !oldOnlyLocalAlphaOk && onlyLocalAlphaOk
onlyLocalValueGone := !onlyLocalAlphaOk && !onlyLocalBetaOk && oldOnlyLocalAlphaOk
hcOnlyLocalMismatch := onlyLocalAlphaOk && healthCheckBetaOk
if onlyLocalValueNew || onlyLocalValueChanged || onlyLocalValueGone || hcOnlyLocalMismatch {
fieldPath := field.NewPath("metadata", "annotations").Key(apiservice.AlphaAnnotationExternalTraffic)
msg := fmt.Sprintf("please replace the alpha annotation with the beta version %v",
apiservice.BetaAnnotationExternalTraffic)
allErrs = append(allErrs, field.Invalid(fieldPath, apiservice.AlphaAnnotationExternalTraffic, msg))
}
return
}
// ValidateServiceUpdate tests if required fields in the service are set during an update
func ValidateServiceUpdate(service, oldService *api.Service) field.ErrorList {
allErrs := ValidateObjectMetaUpdate(&service.ObjectMeta, &oldService.ObjectMeta, field.NewPath("metadata"))
@ -2578,7 +2642,8 @@ func ValidateServiceUpdate(service, oldService *api.Service) field.ErrorList {
// TODO(freehan): allow user to update loadbalancerSourceRanges
allErrs = append(allErrs, ValidateImmutableField(service.Spec.LoadBalancerSourceRanges, oldService.Spec.LoadBalancerSourceRanges, field.NewPath("spec", "loadBalancerSourceRanges"))...)
allErrs = append(allErrs, ValidateService(service)...)
allErrs = append(allErrs, validateServiceFields(service)...)
allErrs = append(allErrs, validateServiceAnnotations(service, oldService)...)
return allErrs
}

45
pkg/api/validation/validation_test.go
View File

@ -5207,6 +5207,13 @@ func TestValidateService(t *testing.T) {
},
numErrs: 1,
},
{
name: "LoadBalancer disallows onlyLocal alpha annotations",
tweakSvc: func(s *api.Service) {
s.Annotations[service.AlphaAnnotationExternalTraffic] = service.AnnotationValueExternalTrafficLocal
},
numErrs: 1,
},
}
for _, tc := range testCases {
@ -6474,6 +6481,44 @@ func TestValidateServiceUpdate(t *testing.T) {
},
numErrs: 1,
},
{
name: "Service disallows removing one onlyLocal alpha annotation",
tweakSvc: func(oldSvc, newSvc *api.Service) {
oldSvc.Annotations[service.AlphaAnnotationExternalTraffic] = service.AnnotationValueExternalTrafficLocal
oldSvc.Annotations[service.AlphaAnnotationHealthCheckNodePort] = "3001"
},
numErrs: 2,
},
{
name: "Service disallows modifying onlyLocal alpha annotations",
tweakSvc: func(oldSvc, newSvc *api.Service) {
oldSvc.Annotations[service.AlphaAnnotationExternalTraffic] = service.AnnotationValueExternalTrafficLocal
oldSvc.Annotations[service.AlphaAnnotationHealthCheckNodePort] = "3001"
newSvc.Annotations[service.AlphaAnnotationExternalTraffic] = service.AnnotationValueExternalTrafficGlobal
newSvc.Annotations[service.AlphaAnnotationHealthCheckNodePort] = oldSvc.Annotations[service.AlphaAnnotationHealthCheckNodePort]
},
numErrs: 1,
},
{
name: "Service disallows promoting one of the onlyLocal pair to beta",
tweakSvc: func(oldSvc, newSvc *api.Service) {
oldSvc.Annotations[service.AlphaAnnotationExternalTraffic] = service.AnnotationValueExternalTrafficLocal
oldSvc.Annotations[service.AlphaAnnotationHealthCheckNodePort] = "3001"
newSvc.Annotations[service.BetaAnnotationExternalTraffic] = service.AnnotationValueExternalTrafficGlobal
newSvc.Annotations[service.AlphaAnnotationHealthCheckNodePort] = oldSvc.Annotations[service.AlphaAnnotationHealthCheckNodePort]
},
numErrs: 1,
},
{
name: "Service allows changing both onlyLocal annotations from alpha to beta",
tweakSvc: func(oldSvc, newSvc *api.Service) {
oldSvc.Annotations[service.AlphaAnnotationExternalTraffic] = service.AnnotationValueExternalTrafficLocal
oldSvc.Annotations[service.AlphaAnnotationHealthCheckNodePort] = "3001"
newSvc.Annotations[service.BetaAnnotationExternalTraffic] = service.AnnotationValueExternalTrafficLocal
newSvc.Annotations[service.BetaAnnotationHealthCheckNodePort] = oldSvc.Annotations[service.AlphaAnnotationHealthCheckNodePort]
},
numErrs: 0,
},
}
for _, tc := range testCases {

13
pkg/registry/core/service/rest.go
View File

@ -352,14 +352,11 @@ func (rs *REST) healthCheckNodePortUpdate(oldService, service *api.Service) (boo
}
case oldServiceHasHealthCheckNodePort && assignHealthCheckNodePort:
for _, annotation := range []string{
apiservice.AlphaAnnotationHealthCheckNodePort, apiservice.BetaAnnotationHealthCheckNodePort} {
if _, ok := service.Annotations[annotation]; !ok {
glog.Warningf("Attempt to delete health check node port annotation DENIED")
el := field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations"),
annotation, "Cannot delete healthcheck nodePort annotation")}
return false, errors.NewInvalid(api.Kind("Service"), service.Name, el)
}
if _, ok := service.Annotations[apiservice.BetaAnnotationHealthCheckNodePort]; !ok {
glog.Warningf("Attempt to delete health check node port annotation DENIED")
el := field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations"),
apiservice.BetaAnnotationHealthCheckNodePort, "Cannot delete healthcheck nodePort annotation")}
return false, errors.NewInvalid(api.Kind("Service"), service.Name, el)
}
if oldHealthCheckNodePort != requestedHealthCheckNodePort {
glog.Warningf("Attempt to change value of health check node port annotation DENIED")

8
test/e2e/service.go
View File

@ -1095,7 +1095,7 @@ var _ = framework.KubeDescribe("ESIPP [Slow][Feature:ExternalTrafficLocalOnly]",
c = f.Client
cs = f.ClientSet
if nodes := framework.GetReadySchedulableNodesOrDie(c); len(nodes.Items) > largeClusterMinNodesNumber {
if nodes := framework.GetReadySchedulableNodesOrDie(cs); len(nodes.Items) > largeClusterMinNodesNumber {
loadBalancerCreateTimeout = loadBalancerCreateTimeoutLarge
}
})
@ -2104,7 +2104,7 @@ func (j *ServiceTestJig) createOnlyLocalNodePortService(namespace, serviceName s
svc := j.CreateTCPServiceOrFail(namespace, func(svc *api.Service) {
svc.Spec.Type = api.ServiceTypeNodePort
svc.ObjectMeta.Annotations = map[string]string{
service.AlphaAnnotationExternalTraffic: service.AnnotationValueExternalTrafficLocal}
service.BetaAnnotationExternalTraffic: service.AnnotationValueExternalTrafficLocal}
svc.Spec.Ports = []api.ServicePort{{Protocol: "TCP", Port: 80}}
})
@ -2126,7 +2126,7 @@ func (j *ServiceTestJig) createOnlyLocalLoadBalancerService(namespace, serviceNa
// We need to turn affinity off for our LB distribution tests
svc.Spec.SessionAffinity = api.ServiceAffinityNone
svc.ObjectMeta.Annotations = map[string]string{
service.AlphaAnnotationExternalTraffic: service.AnnotationValueExternalTrafficLocal}
service.BetaAnnotationExternalTraffic: service.AnnotationValueExternalTrafficLocal}
svc.Spec.Ports = []api.ServicePort{{Protocol: "TCP", Port: 80}}
})
@ -2171,7 +2171,7 @@ func (j *ServiceTestJig) getEndpointNodes(svc *api.Service) map[string][]string
// getNodes returns the first maxNodesForTest nodes. Useful in large clusters
// where we don't eg: want to create an endpoint per node.
func (j *ServiceTestJig) getNodes(maxNodesForTest int) (nodes *api.NodeList) {
nodes = framework.GetReadySchedulableNodesOrDie(j.Client)
nodes = framework.GetReadySchedulableNodesOrDie(j.ClientSet)
if len(nodes.Items) <= maxNodesForTest {
maxNodesForTest = len(nodes.Items)
}