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

Make validation check for legal service port numbers. #1006

Merged
merged 2 commits into from
Aug 25, 2014
Merged

Make validation check for legal service port numbers. #1006

merged 2 commits into from
Aug 25, 2014

Conversation

satnam6502
Copy link
Contributor

I thought I would have a crack at the bug "Service port validation #974".
This request only deals with checking valid port numbers in the Port field of Service but does not deal with port number clashes between multiple services.

@@ -260,6 +260,10 @@ func ValidateService(service *Service) errs.ErrorList {
} else if !util.IsDNS952Label(service.ID) {
allErrs = append(allErrs, errs.NewInvalid("Service.ID", service.ID))
}
// Make sure the service port number is valid.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment seems unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Personally, I think the comment is helpful, but I am removing it as per your suggestion.

@lavalamp
Copy link
Member

A couple nits, otherwise LGTM

@brendandburns
Copy link
Contributor

LGTM (assuming fixes to @lavalamp's comments)

@satnam6502
Copy link
Contributor Author

OK: I'm done with my changes in response to (most) of the comments above. If there is a strong desire to change this to a table driven test then I can do that. Thank you.

@lavalamp
Copy link
Member

Thanks fort the change!

lavalamp added a commit that referenced this pull request Aug 25, 2014
Make validation check for legal service port numbers.
@lavalamp lavalamp merged commit b8c57ea into kubernetes:master Aug 25, 2014
vishh pushed a commit to vishh/kubernetes that referenced this pull request Apr 6, 2016
Fix goroutine leak in docker fs handler logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants