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.

Sign up for GitHub

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

Jump to bottom

Add NetworkPolicy API Resource #25638

Merged
merged 1 commit into from
May 21, 2016
Merged

Add NetworkPolicy API Resource #25638

merged 1 commit into from
May 21, 2016
+7,312 −589

Conversation

caseydavenport
Copy link
Member

@caseydavenport caseydavenport commented May 16, 2016 •
edited
Loading

API implementation of #24154

Still to do:

  • Get it working (See comments)
  • Make sure user-facing comments are correct.
  • Update naming in response to Network policy proposal #24154
  • kubectl / client support
  • Release note.
Implement NetworkPolicy v1beta1 API object / client support.

Next Steps:

  • UTs in separate PR.
  • e2e test in separate PR.
  • make Ports + From pointers to slices (TODOs in code - to be done when auto-gen is fixed)

CC @thockin

Analytics

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@caseydavenport
Copy link
Member Author

caseydavenport commented May 16, 2016 •
edited
Loading

This is currently "almost working". I can see the new policy/v1beta1/networkpolicy resource, but I can't POST to the API endpoint. I haven't yet been able to track down the issue. I get the following, even though the json I've passed in does define "metadata.name".

$ curl -H "Content-Type: application/json" -XPOST -d @test-policy.json http://localhost:8080/apis/policy/v1beta1/namespaces/default/networkpolicy
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {},
  "status": "Failure",
  "message": "NetworkPolicy.policy \"\" is invalid: metadata.name: Required value: name or generateName is required",
  "reason": "Invalid",
  "details": {
    "group": "policy",
    "kind": "NetworkPolicy",
    "causes": [
      {
        "reason": "FieldValueRequired",
        "message": "Required value: name or generateName is required",
        "field": "metadata.name"
      }
    ]
  },
  "code": 422
}

$ kubectl create -f test-policy.yaml
The NetworkPolicy "" is invalid.
metadata.name: Required value: name or generateName is required

@caseydavenport caseydavenport mentioned this pull request May 16, 2016
Merged
@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/new-api size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 16, 2016
@zhouhaibing089 zhouhaibing089 mentioned this pull request May 16, 2016
Closed
thockin
thockin reviewed May 16, 2016
View reviewed changes
pkg/apis/policy/types.go

// This NetworkPolicyIngressRule matches traffic if and only if the traffic matches both Ports AND From.
type NetworkPolicyIngressRule struct {
// List of ports which should be made accessible on the pods selected by PodSelector.
Copy link
Member

Choose a reason for hiding this comment

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

s/by PodSelector/for this rule/ - don't reference fields outside the current object.

Copy link
Member

Choose a reason for hiding this comment

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

The proposal doc had lots of good comments - what happened? Specifically "if not provided, match all" and "if empty match none", although we can't really implement that. Go conflates empty and non-specified with "omitempty". I think we can only say that unspecified or empty means no ports are allowed.

We can make it a pointer to slice, if we want to keep the semantic. https://play.golang.org/p/-yCdDclY7_

@caseydavenport
Copy link
Member Author

@thockin - Found the problem. Autogen seems to have messed up - types.generated.go is missing after that latest rebase....

Fix on the way.

@googlebot
Copy link

CLAs look good, thanks!

@thockin
Copy link
Member

thockin commented May 20, 2016

go home @googlebot, you're drunk

@caseydavenport caseydavenport force-pushed the cd-network-policy-api branch from a9b51d7 to 47248f3 Compare May 20, 2016 02:03
@googlebot
Copy link

CLAs look good, thanks!

@caseydavenport
Copy link
Member Author

My CLA is so fine.

Hopefully the code is this time as well...

@googlebot
Copy link

CLAs look good, thanks!

@k8s-github-robot k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 20, 2016
@thockin thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 20, 2016
@thockin
Copy link
Member

thockin commented May 20, 2016

@k8s-bot test this: github issue #IGNORE

I have no idea which flake it is - the logs are utterly useless

2 similar comments
@thockin
Copy link
Member

thockin commented May 20, 2016

@k8s-bot test this: github issue #IGNORE

I have no idea which flake it is - the logs are utterly useless

@thockin
Copy link
Member

thockin commented May 20, 2016

@k8s-bot test this: github issue #IGNORE

I have no idea which flake it is - the logs are utterly useless

@smarterclayton
Copy link
Contributor

Tagged with ok-to-merge

@k8s-bot
Copy link

k8s-bot commented May 21, 2016

GCE e2e build/test passed for commit 47248f3.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit d06fe0c into kubernetes:master May 21, 2016
@caseydavenport caseydavenport mentioned this pull request May 23, 2016
Closed
8 tasks
This was referenced Jun 8, 2016
Merged
Closed
@robbrockbank robbrockbank mentioned this pull request Jun 15, 2016
Closed
@caseydavenport caseydavenport mentioned this pull request Jul 8, 2016
Closed
@ozdanborne ozdanborne mentioned this pull request Oct 27, 2016
Merged
k8s-github-robot pushed a commit that referenced this pull request May 2, 2017
Merge pull request #35660 from djosborne/e2e-network-policy
11c5d12 
Automatic merge from submit-queue

Add e2e network policy tests.

**What this PR does / why we need it**:
Add set of e2e tests for Network Policy. This has succesfully run againast a deployment using Calico as the network policy provider.

Specifically, adds a new e2e test file (/test/e2e/network_policy.go) which tests TCP connectivity between pods with isolation.

See #25638 for PR that added NetworkPolicy resource.

This PR is a replacement for #27447 .
@caseydavenport caseydavenport deleted the cd-network-policy-api branch June 20, 2017 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@thockin thockin

Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.
8 participants
@caseydavenport @googlebot @thockin @smarterclayton @k8s-bot @k8s-github-robot @feiskyer @bgrant0607