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

Added option for backend to have a vhost override #103

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added option for backend to have a vhost override #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions libamqpprox/amqpprox_backend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Backend::Backend(const std::string &name,
const std::string &datacenterTag,
const std::string &host,
const std::string &ip,
const std::string &virtualHost,
int port,
bool proxyEnabled,
bool tlsEnabled,
Expand All @@ -33,6 +34,7 @@ Backend::Backend(const std::string &name,
, d_datacenterTag(datacenterTag)
, d_host(host)
, d_ip(ip)
, d_virtualHost(virtualHost)
, d_port(port)
, d_proxyProtocolEnabled(proxyEnabled)
, d_tlsEnabled(tlsEnabled)
Expand All @@ -45,6 +47,7 @@ Backend::Backend()
, d_datacenterTag("")
, d_host("")
, d_ip("")
, d_virtualHost("")
, d_port(0)
, d_proxyProtocolEnabled(false)
, d_tlsEnabled(false)
Expand All @@ -56,7 +59,7 @@ std::ostream &operator<<(std::ostream &os, const Backend &backend)
{
os << backend.name() << " (" << backend.datacenterTag()
<< "): " << backend.host() << " " << backend.ip() << ":"
<< backend.port();
<< backend.port() << " <" << backend.virtualHost() << ">";
if (backend.proxyProtocolEnabled()) {
os << " " << Constants::proxyProtocolV1Enabled();
}
Expand All @@ -71,7 +74,7 @@ bool operator==(const Backend &lhs, const Backend &rhs)
return (lhs.name() == rhs.name() &&
lhs.datacenterTag() == rhs.datacenterTag() &&
lhs.host() == rhs.host() && lhs.ip() == rhs.ip() &&
lhs.port() == rhs.port() &&
lhs.virtualHost() == rhs.virtualHost() && lhs.port() == rhs.port() &&
lhs.proxyProtocolEnabled() == rhs.proxyProtocolEnabled() &&
lhs.tlsEnabled() == rhs.tlsEnabled());
}
Expand Down
20 changes: 20 additions & 0 deletions libamqpprox/amqpprox_backend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class Backend {
std::string d_datacenterTag;
std::string d_host;
std::string d_ip;
std::string d_virtualHost;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest we use std::optional<std::string> here - mainly for the benefit of indicating that it may not be set.

int d_port;
bool d_proxyProtocolEnabled;
bool d_tlsEnabled;
Expand All @@ -40,14 +41,28 @@ class Backend {
const std::string &datacenterTag,
const std::string &host,
const std::string &ip,
const std::string &virtualHost,
int port,
bool proxyProtocolEnabled = false,
bool tlsEnabled = false,
bool dnsBasedEntry = false);

Backend(const std::string &name,
const std::string &datacenterTag,
const std::string &host,
const std::string &ip,
int port,
bool proxyProtocolEnabled = false,
bool tlsEnabled = false,
bool dnsBasedEntry = false)
: Backend(name, datacenterTag, host, ip, "", port, proxyProtocolEnabled, tlsEnabled, dnsBasedEntry)
{
}

Backend();

inline const std::string &host() const;
inline const std::string &virtualHost() const;
inline const std::string &ip() const;
inline int port() const;
inline const std::string &datacenterTag() const;
Expand All @@ -62,6 +77,11 @@ inline const std::string &Backend::host() const
return d_host;
}

inline const std::string &Backend::virtualHost() const
{
return d_virtualHost;
}

inline const std::string &Backend::ip() const
{
return d_ip;
Expand Down
12 changes: 10 additions & 2 deletions libamqpprox/amqpprox_backendcontrolcommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ std::string BackendControlCommand::commandVerb() const

std::string BackendControlCommand::helpText() const
{
return "(ADD name datacenter host port [SEND-PROXY] [TLS] | ADD_DNS name "
"datacenter address port [SEND-PROXY] [TLS] | DELETE name | "
return "(ADD name datacenter host[/vhost] port [SEND-PROXY] [TLS] | ADD_DNS name "
"datacenter address[/vhost] port [SEND-PROXY] [TLS] | DELETE name | "
"PRINT) - Change backend servers";
}

Expand All @@ -63,6 +63,7 @@ void BackendControlCommand::handleCommand(const std::string & /* command */,
std::string name;
std::string datacenter;
std::string host;
std::string virtualHost;
int port = 0;
std::string arg1, arg2;
iss >> name;
Expand All @@ -75,6 +76,12 @@ void BackendControlCommand::handleCommand(const std::string & /* command */,
boost::to_upper(arg2);

if (!name.empty() && !datacenter.empty() && !host.empty() && port) {
std::string::size_type vhostPos = host.find("/");
if (vhostPos != std::string::npos) {
virtualHost = host.substr(vhostPos + 1);
host = host.substr(0, vhostPos);
}

std::string ip;
if (!isDns) {
auto &ioContext = controlHandle->ioContext();
Expand All @@ -99,6 +106,7 @@ void BackendControlCommand::handleCommand(const std::string & /* command */,
datacenter,
host,
ip,
virtualHost,
port,
isSendProxy,
isSecure,
Expand Down
6 changes: 5 additions & 1 deletion libamqpprox/amqpprox_connector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,11 @@ void Connector::receive(const Method &method, FlowType direction)
LOG_TRACE << "Server Tune: " << d_receivedTune;

sendResponse(d_tuneOk, false);
sendResponse(d_open, false);

methods::Open openCopy = d_open;
openCopy.setVirtualHost(d_sessionState_p->getBackendVirtualHost());
sendResponse(openCopy, false);
Comment on lines +224 to +226
Copy link
Contributor

Choose a reason for hiding this comment

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

The only property which is encoded/decoded in amqpprox on Open is the vhost so copying the inbound open is a little pointless now, I feel it's neater to keep the Open object immutable and add a constructor which takes a vhost string. I think we could go with something like this, and ditch the d_open property on the Connector

Suggested change
methods::Open openCopy = d_open;
openCopy.setVirtualHost(d_sessionState_p->getBackendVirtualHost());
sendResponse(openCopy, false);
methods::Open openMethod{d_sessionState_p->getBackendVirtualHost()};
sendResponse(openMethod, false);

Copy link
Author

Choose a reason for hiding this comment

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

I wasnt sure what else used d_open so making a copy was 'safer'. Open to doing what ever way makes more sense


d_state = State::OPEN_SENT;
} break;
case State::OPEN_SENT: {
Expand Down
4 changes: 4 additions & 0 deletions libamqpprox/amqpprox_methods_open.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ class Open {
public:
const std::string &virtualHost() const { return d_virtualHost; }

void setVirtualHost(const std::string& virtualHost) {
d_virtualHost = virtualHost;
}

/**
* \brief Decode specified buffer and copy the data into open method
*/
Expand Down
8 changes: 7 additions & 1 deletion libamqpprox/amqpprox_session.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,12 @@ void Session::attemptConnection(
auto currentBackend =
connectionManager->getConnection(d_egressRetryCounter);

if (currentBackend != nullptr) {
d_sessionState.setBackendVirtualHost(currentBackend->virtualHost());
} else {
d_sessionState.setBackendVirtualHost("");
}

// With Boost ASIO it sometimes on Linux returns a good error code,
// but no items in the list. This catches this case as well as the
// regular error return.
Expand Down Expand Up @@ -550,7 +556,7 @@ void Session::establishConnection()

// Initialize auth request data
authproto::AuthRequest authRequestData;
authRequestData.set_vhostname(d_sessionState.getVirtualHost());
authRequestData.set_vhostname(d_sessionState.getBackendVirtualHost());
authproto::SASL *saslPtr = authRequestData.mutable_authdata();
saslPtr->set_authmechanism(sasl.first);
saslPtr->set_credentials(sasl.second);
Expand Down
1 change: 1 addition & 0 deletions libamqpprox/amqpprox_sessionstate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ std::ostream &operator<<(std::ostream &os, const SessionState &state)

os << std::setw(7) << state.id() << ": "
<< "vhost=" << state.getVirtualHost() << " "
<< "vhost-back=" << state.getBackendVirtualHost() << " "
<< ", "
<< (state.getDisconnectType() ==
SessionState::DisconnectType::NOT_DISCONNECTED
Expand Down
17 changes: 17 additions & 0 deletions libamqpprox/amqpprox_sessionstate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ class SessionState {
std::atomic<bool> d_ingressSecured;
std::atomic<bool> d_limitedConnection;
std::string d_virtualHost;
std::string d_backendVirtualHost;
DisconnectType d_disconnectedStatus;
uint64_t d_id;
mutable std::mutex d_lock;
Expand Down Expand Up @@ -209,6 +210,10 @@ class SessionState {
*/
inline const std::string &getVirtualHost() const;

inline void setBackendVirtualHost(const std::string& virtualHost);

inline const std::string &getBackendVirtualHost() const;

/**
* \return the state(paused/unpaused) of the virtual host
*/
Expand Down Expand Up @@ -280,6 +285,18 @@ inline const std::string &SessionState::getVirtualHost() const
return d_virtualHost;
}

inline void SessionState::setBackendVirtualHost(const std::string& virtualHost)
{
std::lock_guard<std::mutex> lg(d_lock);
d_backendVirtualHost = virtualHost;
}

inline const std::string &SessionState::getBackendVirtualHost() const
{
std::lock_guard<std::mutex> lg(d_lock);
return d_backendVirtualHost.empty() ? d_virtualHost : d_backendVirtualHost;
}

inline bool SessionState::getPaused() const
{
return d_paused;
Expand Down