Skip to content

Commit

Permalink
ceanup: remove istio related code
Browse files Browse the repository at this point in the history
We no longer build proxy images for istio, better just clean it up.

Hence, as an additional follow up to #448, this commit removes the
Istio sidecar specific implementations (socket mark, ...).

Signed-off-by: Marco Hofstetter <[email protected]>
  • Loading branch information
mhofstetter committed Mar 18, 2024
1 parent 1e8408e commit abd9fb5
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 36 deletions.
20 changes: 9 additions & 11 deletions cilium/bpf_metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -407,17 +407,15 @@ bool Config::getMetadata(Network::ConnectionSocket& socket) {
// Pass the metadata to an Envoy socket option we can retrieve later in other
// Cilium filters.
uint32_t mark = 0;
if (!npmap_->is_sidecar_) {
// Mark with source endpoint ID for east/west l7 LB. This causes the upstream packets to be
// processed by the the source endpoint's policy enforcement in the datapath.
if (east_west_l7_lb) {
mark = 0x0900 | endpoint_id << 16;
} else {
// Mark with source identity
uint32_t cluster_id = (source_identity >> 16) & 0xFF;
uint32_t identity_id = (source_identity & 0xFFFF) << 16;
mark = ((is_ingress_) ? 0x0A00 : 0x0B00) | cluster_id | identity_id;
}
// Mark with source endpoint ID for east/west l7 LB. This causes the upstream packets to be
// processed by the the source endpoint's policy enforcement in the datapath.
if (east_west_l7_lb) {
mark = 0x0900 | endpoint_id << 16;
} else {
// Mark with source identity
uint32_t cluster_id = (source_identity >> 16) & 0xFF;
uint32_t identity_id = (source_identity & 0xFFFF) << 16;
mark = ((is_ingress_) ? 0x0A00 : 0x0B00) | cluster_id | identity_id;
}
socket.addOption(std::make_shared<Cilium::SocketOption>(
policy, mark, ingress_source_identity, source_identity, is_ingress_, is_l7lb_, dip->port(),
Expand Down
4 changes: 2 additions & 2 deletions cilium/network_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ Network::FilterStatus Instance::onNewConnection() {
}

// Pass metadata from tls_inspector to the filterstate, if any & not already
// set via upstream cluster config, but not in a sidecar, which have no mark
if (sni != "" && !option->isSidecar()) {
// set via upstream cluster config.
if (sni != "") {
auto filterState = conn.streamInfo().filterState();
auto have_sni =
filterState->hasData<Network::UpstreamServerName>(Network::UpstreamServerName::key());
Expand Down
3 changes: 1 addition & 2 deletions cilium/network_policy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1074,8 +1074,7 @@ NetworkPolicyMap::NetworkPolicyMap(Server::Configuration::FactoryContext& contex
context.getServerFactoryContext(),
context.getTransportSocketFactoryContext().sslContextManager(), *scope_,
context.getServerFactoryContext().clusterManager(),
context.messageValidationContext().dynamicValidationVisitor())),
is_sidecar_(context.localInfo().nodeName().rfind("sidecar~", 0) == 0) {
context.messageValidationContext().dynamicValidationVisitor())) {
// Use listener init manager for the first initialization
transport_factory_context_->setInitManager(context.initManager());
context.initManager().add(init_target_);
Expand Down
3 changes: 0 additions & 3 deletions cilium/network_policy.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,6 @@ class NetworkPolicyMap : public Singleton::Instance,

ProtobufTypes::MessagePtr dumpNetworkPolicyConfigs(const Matchers::StringMatcher& name_matcher);
Server::ConfigTracker::EntryOwnerPtr config_tracker_entry_;

public:
const bool is_sidecar_;
};

} // namespace Cilium
Expand Down
10 changes: 0 additions & 10 deletions cilium/socket_option.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ class SocketMarkOption : public Network::Socket::Option,

bool setOption(Network::Socket& socket,
envoy::config::core::v3::SocketOption::SocketState state) const override {
// sidecars do not have mark
if (mark_ == 0) {
return true;
}
// Only set the option once per socket
if (state != envoy::config::core::v3::SocketOption::STATE_PREBIND) {
ENVOY_LOG(trace, "Skipping setting socket ({}) option SO_MARK, state != STATE_PREBIND",
Expand Down Expand Up @@ -171,10 +167,6 @@ class SocketMarkOption : public Network::Socket::Option,
}

void hashKey(std::vector<uint8_t>& key) const override {
// sidecars have no mark
if (mark_ == 0) {
return;
}
// Source address is more specific than policy ID. If using an original
// source address, we do not need to also add the source security ID to the
// hash key. Note that since the identity is 3 bytes it will not collide
Expand All @@ -198,8 +190,6 @@ class SocketMarkOption : public Network::Socket::Option,

bool isSupported() const override { return true; }

bool isSidecar() const { return mark_ == 0; }

uint32_t identity_;
uint32_t mark_;
Network::Address::InstanceConstSharedPtr original_source_address_;
Expand Down
5 changes: 2 additions & 3 deletions proxylib/proxylib/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,12 @@ var (
)

func NewInstance(nodeID string, accessLogger AccessLogger) *Instance {

instanceId++

if nodeID == "" {
nodeID = fmt.Sprintf("host~127.0.0.2~libcilium-%d~localdomain", instanceId)
}

// TODO: Sidecar instance id needs to be different.
ins := &Instance{
id: instanceId,
openCount: 1,
Expand All @@ -72,7 +70,8 @@ func NewInstance(nodeID string, accessLogger AccessLogger) *Instance {
// OpenInstance creates a new instance or finds an existing one with equivalent parameters.
// returns the instance id.
func OpenInstance(nodeID string, xdsPath string, newPolicyClient func(path, nodeID string, updater PolicyUpdater) PolicyClient,
accessLogPath string, newAccessLogger func(accessLogPath string) AccessLogger) uint64 {
accessLogPath string, newAccessLogger func(accessLogPath string) AccessLogger,
) uint64 {
mutex.Lock()
defer mutex.Unlock()

Expand Down
3 changes: 1 addition & 2 deletions proxylib/proxylib/policymap.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,7 @@ func (p *PortNetworkPolicies) Matches(port, remoteId uint32, l7 interface{}) boo
}

// No policy for the port was found. Cilium always creates a policy for redirects it
// creates, so the host proxy never gets here. Sidecar gets all the traffic, which we need
// to pass through since the bpf datapath already allowed it.
// creates, so the host proxy never gets here.
// TODO: Change back to false only when non-bpf datapath is supported?

// logrus.Debugf("NPDS::PortNetworkPolicies(port=%d, remoteId=%d): allowing traffic on port for which there is no policy, assuming L3/L4 has passed it! (%v)", port, remoteId, p)
Expand Down
4 changes: 1 addition & 3 deletions proxylib/test/accesslog_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"golang.org/x/sys/unix"

cilium "github.com/cilium/proxy/go/cilium/api"

"github.com/cilium/proxy/pkg/inctimer"
"github.com/cilium/proxy/pkg/lock"
)
Expand Down Expand Up @@ -92,8 +91,7 @@ func StartAccessLogServer(accessLogName string, bufSize int) *AccessLogServer {
}
server.listener.SetUnlinkOnClose(true)

// Make the socket accessible by non-root Envoy proxies, e.g. running in
// sidecar containers.
// Make the socket accessible by non-root Envoy proxies.
if err = os.Chmod(accessLogPath, 0777); err != nil {
logrus.Fatalf("Failed to change mode of access log listen socket at %s: %v", accessLogPath, err)
}
Expand Down

0 comments on commit abd9fb5

Please sign in to comment.