-
Notifications
You must be signed in to change notification settings - Fork 6
first attempt to add cross account permissions to aws #24
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| { | ||
| description = "cilium dev environment"; | ||
|
|
||
| inputs = { | ||
| nixpkgs.url = "github:nixos/nixpkgs"; | ||
| nixpkgs-tparse.url = "github:NixOS/nixpkgs/e518d4ad2bcad74f98fec028cf21ce5b1e5020dd"; #revision for tparse | ||
| nixpkgs-ginkgo.url = "github:NixOS/nixpkgs/89f196fe781c53cb50fef61d3063fa5e8d61b6e5"; #revision for ginkgo | ||
| }; | ||
|
|
||
| outputs = { self, nixpkgs, nixpkgs-tparse, nixpkgs-ginkgo }: | ||
| let | ||
| allSystems = [ | ||
| "x86_64-linux" # 64-bit Intel/AMD Linux | ||
| "aarch64-linux" # 64-bit ARM Linux for my pbp | ||
| "x86_64-darwin" # 64-bit Intel macOS | ||
| "aarch64-darwin" # 64-bit ARM macOS | ||
| ]; | ||
|
|
||
| # Helper to provide system-specific attributes | ||
| forAllSystems = f: nixpkgs.lib.genAttrs allSystems (system: f { | ||
| pkgs = import nixpkgs { inherit system; }; | ||
| pkgs-tp = import nixpkgs-tparse { inherit system; }; | ||
| pkgs-g = import nixpkgs-ginkgo { inherit system; }; | ||
| }); | ||
| in { | ||
| # Development env package required. | ||
| devShells = forAllSystems ({ pkgs, pkgs-tp, pkgs-g }: { | ||
| default = pkgs.mkShell { | ||
| packages = with pkgs; [ | ||
| neovim | ||
| go # Go 1.24.4 | ||
| gotools | ||
| llvmPackages_18.clangUseLLVM | ||
| pkgs-g.ginkgo | ||
| golangci-lint | ||
| docker | ||
| docker-compose | ||
| python313Packages.pip | ||
| kubernetes-helm | ||
| kind | ||
| kubectl | ||
| cilium-cli | ||
| pkgs-tp.tparse | ||
| ]; | ||
| }; | ||
| }); | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // Copyright Authors of Cilium | ||
|
|
||
| package eni | ||
|
|
||
| import ( | ||
| "context" | ||
| "log/slog" | ||
| ec2_types "github.com/aws/aws-sdk-go-v2/service/ec2/types" | ||
| eniTypes "github.com/cilium/cilium/pkg/aws/eni/types" | ||
| "github.com/cilium/cilium/pkg/aws/types" | ||
| ipamTypes "github.com/cilium/cilium/pkg/ipam/types" | ||
| "github.com/cilium/cilium/pkg/logging/logfields" | ||
| ) | ||
|
|
||
| // CrossAccountEC2Client splits EC2 API calls between two accounts: | ||
| // - remote: ownwer of the VPC and pod subnets. This handles all ENI lifecycle operations | ||
| // - local: owner of the EC2 instances. This handles all instance-level operations including attachments | ||
| // | ||
| // After CreateNetworkInterface succeeds on the remote client, a | ||
| // CreateNetworkInterfacePermission call grants the local account INSTANCE-ATTACH | ||
| // access so that AttachNetworkInterface can be called from the local account. | ||
| type CrossAccountEC2Client struct { | ||
| logger *slog.Logger | ||
| local EC2API | ||
| remote EC2API | ||
| localAccountID string | ||
| } | ||
|
|
||
| // NewCrossAccountEC2Client constructs a CrossAccountEC2Client. | ||
| // the localAccountID is needed to setup every CreateNetworkInterfacePermission | ||
| func NewCrossAccountEC2Client(logger *slog.Logger, local, remote EC2API, localAccountID string) *CrossAccountEC2Client { | ||
| return &CrossAccountEC2Client{ | ||
| logger: logger, | ||
| local: local, | ||
| remote: remote, | ||
| localAccountID: localAccountID, | ||
| } | ||
| } | ||
|
|
||
| // ********************************************************************************* | ||
| // --- VPC / Subnet / ENI-owner accouint operations → remote (network account) --- | ||
| // ********************************************************************************* | ||
|
|
||
| func (c *CrossAccountEC2Client) GetSubnets(ctx context.Context, vpcID string) (ipamTypes.SubnetMap, error) { | ||
| return c.remote.GetSubnets(ctx, vpcID) | ||
| } | ||
|
|
||
| func (c *CrossAccountEC2Client) GetVpcs(ctx context.Context, vpcID string) (ipamTypes.VirtualNetworkMap, error) { | ||
| return c.remote.GetVpcs(ctx, vpcID) | ||
| } | ||
|
|
||
| func (c *CrossAccountEC2Client) GetRouteTables(ctx context.Context, vpcID string) (ipamTypes.RouteTableMap, error) { | ||
| return c.remote.GetRouteTables(ctx, vpcID) | ||
| } | ||
|
|
||
| //TODO: not sure if I need this | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need this comment? it seems correct to me. |
||
| func (c *CrossAccountEC2Client) GetSecurityGroups(ctx context.Context, vpcID string) (types.SecurityGroupMap, error) { | ||
| return c.remote.GetSecurityGroups(ctx, vpcID) | ||
| } | ||
|
|
||
| func (c *CrossAccountEC2Client) GetDetachedNetworkInterfaces(ctx context.Context, tags ipamTypes.Tags, maxResults int32) ([]string, error) { | ||
| return c.remote.GetDetachedNetworkInterfaces(ctx, tags, maxResults) | ||
| } | ||
|
|
||
| // CreateNetworkInterface creates the ENI in the remote account's subnet, then | ||
| // immediately grants the local account INSTANCE-ATTACH permission so that | ||
| // AttachNetworkInterface (local) can succeed. | ||
| func (c *CrossAccountEC2Client) CreateNetworkInterface(ctx context.Context, toAllocate int32, subnetID, desc string, groups []string, allocatePrefixes bool) (string, *eniTypes.ENI, error) { | ||
| eniID, eni, err := c.remote.CreateNetworkInterface(ctx, toAllocate, subnetID, desc, groups, allocatePrefixes) | ||
| if err != nil { | ||
| return "", nil, err | ||
| } | ||
|
|
||
| if permErr := c.remote.CreateNetworkInterfacePermission(ctx, eniID, c.localAccountID); permErr != nil { | ||
| // Permission grant call failed. Delete the orphaned eni and rethrow | ||
| c.logger.Warn( | ||
| "Failed to grant cross-account ENI attach permission. Deleting orphaned ENI", | ||
| logfields.ENI, eniID, | ||
| logfields.Error, permErr, | ||
| ) | ||
| if delErr := c.remote.DeleteNetworkInterface(ctx, eniID); delErr != nil { | ||
| //TODO: maybe make a bigger deal of this | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A |
||
| c.logger.Warn("Failed to delete orphaned ENI", | ||
| logfields.ENI, eniID, | ||
| logfields.Error, delErr, | ||
| ) | ||
| } | ||
| return "", nil, permErr | ||
| } | ||
|
|
||
| return eniID, eni, nil | ||
| } | ||
|
|
||
| func (c *CrossAccountEC2Client) CreateNetworkInterfacePermission(ctx context.Context, eniID string, accountID string) error { | ||
| return c.remote.CreateNetworkInterfacePermission(ctx, eniID, accountID) | ||
| } | ||
|
|
||
| func (c *CrossAccountEC2Client) DeleteNetworkInterface(ctx context.Context, eniID string) error { | ||
| return c.remote.DeleteNetworkInterface(ctx, eniID) | ||
| } | ||
|
|
||
| func (c *CrossAccountEC2Client) AssignPrivateIpAddresses(ctx context.Context, eniID string, addresses int32) ([]string, error) { | ||
| return c.remote.AssignPrivateIpAddresses(ctx, eniID, addresses) | ||
| } | ||
|
|
||
| func (c *CrossAccountEC2Client) UnassignPrivateIpAddresses(ctx context.Context, eniID string, addresses []string) error { | ||
| return c.remote.UnassignPrivateIpAddresses(ctx, eniID, addresses) | ||
| } | ||
|
|
||
| func (c *CrossAccountEC2Client) AssignENIPrefixes(ctx context.Context, eniID string, prefixes int32) error { | ||
| return c.remote.AssignENIPrefixes(ctx, eniID, prefixes) | ||
| } | ||
|
|
||
| func (c *CrossAccountEC2Client) UnassignENIPrefixes(ctx context.Context, eniID string, prefixes []string) error { | ||
| return c.remote.UnassignENIPrefixes(ctx, eniID, prefixes) | ||
| } | ||
|
|
||
|
|
||
| // ********************************************************************************* | ||
| // --- Instance-owner operations → local --- | ||
| // ********************************************************************************* | ||
|
|
||
| func (c *CrossAccountEC2Client) GetInstance(ctx context.Context, vpcs ipamTypes.VirtualNetworkMap, subnets ipamTypes.SubnetMap, instanceID string) (*ipamTypes.Instance, error) { | ||
| return c.local.GetInstance(ctx, vpcs, subnets, instanceID) | ||
| } | ||
|
|
||
| func (c *CrossAccountEC2Client) GetInstances(ctx context.Context, vpcs ipamTypes.VirtualNetworkMap, subnets ipamTypes.SubnetMap) (*ipamTypes.InstanceMap, error) { | ||
| return c.local.GetInstances(ctx, vpcs, subnets) | ||
| } | ||
|
|
||
| // Needed so we can get max limits by type | ||
| func (c *CrossAccountEC2Client) GetInstanceTypes(ctx context.Context) ([]ec2_types.InstanceTypeInfo, error) { | ||
| return c.local.GetInstanceTypes(ctx) | ||
| } | ||
|
|
||
| func (c *CrossAccountEC2Client) AttachNetworkInterface(ctx context.Context, index int32, instanceID, eniID string) (string, error) { | ||
| return c.local.AttachNetworkInterface(ctx, index, instanceID, eniID) | ||
| } | ||
|
|
||
| func (c *CrossAccountEC2Client) ModifyNetworkInterface(ctx context.Context, eniID, attachmentID string, deleteOnTermination bool) error { | ||
| return c.local.ModifyNetworkInterface(ctx, eniID, attachmentID, deleteOnTermination) | ||
| } | ||
|
|
||
| func (c *CrossAccountEC2Client) AssociateEIP(ctx context.Context, eniID string, eipTags ipamTypes.Tags) (string, error) { | ||
| return c.local.AssociateEIP(ctx, eniID, eipTags) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should it be
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. eip's can be either local or remote for the avg user. we don't allow them at all in rbx, but if we did, we would want them in the clients local account. Otherwise we'd have every customer eip in network and it would get messy |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need
flake.nixandflake.lockin this PR.