BCDevOps / platform-services

Collection of platform related tools and configurations
Apache License 2.0
13 stars 29 forks source link

Create network policy that will control explicit egress from specific pods #181

Closed stewartshea closed 5 years ago

stewartshea commented 5 years ago

As a developer I would like the ability to specify which pods may access the internet and which ones may not.

stewartshea commented 5 years ago

This has been tested/validated as part of #134

A sample policy in the Aporeto yaml looks like the following:

APIVersion: 0
data:
  networkaccesspolicies:
    - action: Reject
      annotations:
        io.aporeto/importhash:
          - '5682509029551086726'
      applyPolicyMode: OutgoingTraffic
      associatedTags:
        - 'aporeto:import:label=test-egress-network-policies'
      logsEnabled: true
      name: test-egress-network-policies
      object:
        - - 'ext:network=any'
      propagate: true
      subject:
        - - $namespace=/bcgov/test/lab-pathfinder/devops-logging
identities:
  - networkaccesspolicy

@jleach the above is a sample of what we need to support in the operator.

stewartshea commented 5 years ago

I think the larger question here is should we deny egress by default and then only explicitly allow it at the namespace/project level, or allow it for all namespaces and then explicitly deny access at the namespace level.

@WadeBarnes @jleach @kelpisland Do you have an opinion or leaning? I feel like some policies may get a little more complicated if we are explicitly stating what should have access to the internet vs what should not. For example, our tools namespaces will be more of a copy/paste exercise if we need to list out all "build pods should have access" and the like.

However, the idea behind zero-trust is that we explicitly specify what is allowed, and everything else is denied by default. This is a balance of perceived complexity and "sensible defaults".

WadeBarnes commented 5 years ago

@stewartshea, I'm a fan of the denied by default approach for the deployment environments, and agree the tools environments need to have more relaxed policies by default.

One benefit I see with having the deployment environments locked down is that it forces the teams to start thinking about their project and security architecture out of the gate. I know this is going to be a point of friction for some less experienced teams, but I think it's a better approach. If things are open by default it's much easier to forget/neglect to lock it down later.

stewartshea commented 5 years ago

I tend to agree for exactly the same reasons, thanks for the weigh in @WadeBarnes. Of course we will allow teams to do more of a "allow all from my namespace" but they will need to create that configuration with intention, and it won't be a cluster-wide policy.

mitovskaol commented 5 years ago

If you care about my 2cents, I also think that the "Default Deny" should be the base network policy at least on the platform level. This considered the security best practice and unless we have a compelling reason to deviate from it, lets stick to it.

stewartshea commented 5 years ago

@mitovskaol thanks, your 2cents is definitely appreciated. I've remove the global egress policy as a base configuration and we will ensure the operator is able to configure this option at the project/namespace level.

jleach commented 5 years ago

@stewartshea Hit a bit of a roadblock on this one. For this I need to let users create a 2nd CRD of (for example) kind: ExternalNetwork. But when I add a 2nd one the Operator can't find it; I can comment out one or the other and then it works but by keeping both one fails.

Reached out to the OperatorSDK community here.

If I don't resolve it shortly we may need to just use a single CRD but put or own kind field in the spec then process it in the playbook accordingly.

Cc. @j-pye

jleach commented 5 years ago

Fixed my issue. Unblocked....

stewartshea commented 5 years ago

@jleach the initial thought I had with this was something closer to this:

source:
  - namespace: somenamespace
destination: 
  - externalNetwork: any

It seems to me that it would be more confusing to have to use multiple definitions, as this implies that we will need to keep doing this once we want to use tags / labels / etc. I feel that jinja templating should be able to help us keep this within a single definition, no?

jleach commented 5 years ago

Closing this issue as #187 deals with the remaining ExternalNetworkPolicy to allow the creation of an external network via the Operator.