Skip to content

Commit

Permalink
Fixes #1705: Remove ability to update or delete router links (#1706)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ganeshmurthy
ganeshmurthy authored Dec 23, 2024
1 parent f2b8092 commit 745ca5b
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 160 deletions.
6 changes: 0 additions & 6 deletions python/skupper_router/management/skrouter.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1392,13 +1392,7 @@
"router.link": {
"description": "Link to another AMQP endpoint: router node, client or other AMQP process.",
"extends": "operationalEntity",
"operations": ["UPDATE"],
"attributes": {
"adminStatus": {
"type": ["enabled", "disabled"],
"default": "enabled",
"update": true
},
"operStatus": {
"type": ["up", "down", "quiescing", "idle"]
},
Expand Down
10 changes: 5 additions & 5 deletions src/router_core/agent.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ static void qdr_manage_create_CT(qdr_core_t *core, qdr_action_t *action, bool di
case QD_ROUTER_CONFIG_AUTO_LINK: qdra_config_auto_link_create_CT(core, name, query, in_body); break;
case QD_ROUTER_CONNECTION: break;
case QD_ROUTER_ROUTER_METRICS: qdr_agent_forbidden(core, query, false); break;
case QD_ROUTER_LINK: break;
case QD_ROUTER_LINK: qdr_agent_forbidden(core, query, false); break;
case QD_ROUTER_ADDRESS: break;
case QD_ROUTER_FORBIDDEN: qdr_agent_forbidden(core, query, false); break;
}
Expand All @@ -451,7 +451,7 @@ static void qdr_manage_delete_CT(qdr_core_t *core, qdr_action_t *action, bool di
case QD_ROUTER_CONFIG_AUTO_LINK: qdra_config_auto_link_delete_CT(core, query, name, identity); break;
case QD_ROUTER_CONNECTION: qdr_agent_forbidden(core, query, false); break;
case QD_ROUTER_ROUTER_METRICS: qdr_agent_forbidden(core, query, false); break;
case QD_ROUTER_LINK: break;
case QD_ROUTER_LINK: qdr_agent_forbidden(core, query, false); break;
case QD_ROUTER_ADDRESS: break;
case QD_ROUTER_FORBIDDEN: qdr_agent_forbidden(core, query, false); break;
}
Expand All @@ -469,15 +469,15 @@ static void qdr_manage_update_CT(qdr_core_t *core, qdr_action_t *action, bool di
qd_parsed_field_t *in_body = action->args.agent.in_body;

if (!discard) {
switch (query->entity_type) {
switch (query->entity_type) {
case QD_ROUTER_CONFIG_ADDRESS: break;
case QD_ROUTER_CONFIG_AUTO_LINK: break;
case QD_ROUTER_CONNECTION: qdra_connection_update_CT(core, name, identity, query, in_body); break;
case QD_ROUTER_LINK: qdra_link_update_CT(core, name, identity, query, in_body); break;
case QD_ROUTER_LINK: qdr_agent_forbidden(core, query, false); break;
case QD_ROUTER_ADDRESS: break;
case QD_ROUTER_ROUTER_METRICS: qdr_agent_forbidden(core, query, false); break;
case QD_ROUTER_FORBIDDEN: qdr_agent_forbidden(core, query, false); break;
}
}
}

qdr_field_free(action->args.agent.name);
Expand Down
161 changes: 16 additions & 145 deletions src/router_core/agent_link.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,22 @@
#define QDR_LINK_UNSETTLED_COUNT 9
#define QDR_LINK_DELIVERY_COUNT 10
#define QDR_LINK_CONNECTION_ID 11
#define QDR_LINK_ADMIN_STATE 12
#define QDR_LINK_OPER_STATE 13
#define QDR_LINK_PRESETTLED_COUNT 14
#define QDR_LINK_DROPPED_PRESETTLED_COUNT 15
#define QDR_LINK_ACCEPTED_COUNT 16
#define QDR_LINK_REJECTED_COUNT 17
#define QDR_LINK_RELEASED_COUNT 18
#define QDR_LINK_MODIFIED_COUNT 19
#define QDR_LINK_DELAYED_1SEC 20
#define QDR_LINK_DELAYED_10SEC 21
#define QDR_LINK_DELIVERIES_STUCK 22
#define QDR_LINK_OPEN_MOVED_STREAMS 23
#define QDR_LINK_INGRESS_HISTOGRAM 24
#define QDR_LINK_PRIORITY 25
#define QDR_LINK_SETTLE_RATE 26
#define QDR_LINK_CREDIT_AVAILABLE 27
#define QDR_LINK_ZERO_CREDIT_SECONDS 28
#define QDR_LINK_OPER_STATE 12
#define QDR_LINK_PRESETTLED_COUNT 13
#define QDR_LINK_DROPPED_PRESETTLED_COUNT 14
#define QDR_LINK_ACCEPTED_COUNT 15
#define QDR_LINK_REJECTED_COUNT 16
#define QDR_LINK_RELEASED_COUNT 17
#define QDR_LINK_MODIFIED_COUNT 18
#define QDR_LINK_DELAYED_1SEC 19
#define QDR_LINK_DELAYED_10SEC 20
#define QDR_LINK_DELIVERIES_STUCK 21
#define QDR_LINK_OPEN_MOVED_STREAMS 22
#define QDR_LINK_INGRESS_HISTOGRAM 23
#define QDR_LINK_PRIORITY 24
#define QDR_LINK_SETTLE_RATE 25
#define QDR_LINK_CREDIT_AVAILABLE 26
#define QDR_LINK_ZERO_CREDIT_SECONDS 27

const char *qdr_link_columns[] =
{"name",
Expand All @@ -65,7 +64,6 @@ const char *qdr_link_columns[] =
"unsettledCount",
"deliveryCount",
"connectionId", // The connection id of the owner connection
"adminStatus",
"operStatus",
"presettledCount",
"droppedPresettledCount",
Expand Down Expand Up @@ -174,11 +172,6 @@ static void qdr_agent_write_column_CT(qdr_core_t *core, qd_composed_field_t *bod
break;
}

case QDR_LINK_ADMIN_STATE:
text = link->admin_enabled ? "enabled" : "disabled";
qd_compose_insert_string(body, text);
break;

case QDR_LINK_OPER_STATE:
switch (link->oper_status) {
case QDR_LINK_OPER_UP: text = "up"; break;
Expand Down Expand Up @@ -388,125 +381,3 @@ void qdra_link_get_next_CT(qdr_core_t *core, qdr_query_t *query)
qdr_agent_enqueue_response_CT(core, query);
}


static void qdr_manage_write_response_map_CT(qdr_core_t *core, qd_composed_field_t *body, qdr_link_t *link)
{
qd_compose_start_map(body);

for(int i = 0; i < QDR_LINK_COLUMN_COUNT; i++) {
qd_compose_insert_string(body, qdr_link_columns[i]);
qdr_agent_write_column_CT(core, body, i, link);
}

qd_compose_end_map(body);
}


static qdr_link_t *qdr_link_find_by_identity(qdr_core_t *core, qd_iterator_t *identity)
{
if (!identity)
return 0;

qdr_link_t *link = DEQ_HEAD(core->open_links);

while(link) {
char id[100];
if (link->identity) {
snprintf(id, 100, "%"PRId64, link->identity);
if (qd_iterator_equal(identity, (const unsigned char *)id))
break;
}
link = DEQ_NEXT(link);
}

return link;

}


static qdr_link_t *qdr_link_find_by_name(qdr_core_t *core, qd_iterator_t *name)
{
if(!name)
return 0;

qdr_link_t *link = DEQ_HEAD(core->open_links);

while(link) {
if (link->name && qd_iterator_equal(name, (const unsigned char *)link->name))
break;
link = DEQ_NEXT(link);
}

return link;
}


static void qdra_link_update_set_status(qdr_core_t *core, qdr_query_t *query, qdr_link_t *link)
{
if (link) {
//link->admin_state = qd_iterator_copy(adm_state);
qdr_manage_write_response_map_CT(core, query->body, link);
query->status = QD_AMQP_OK;
}
else {
query->status = QD_AMQP_NOT_FOUND;
qd_compose_start_map(query->body);
qd_compose_end_map(query->body);
}
}

static void qdra_link_set_bad_request(qdr_query_t *query)
{
query->status = QD_AMQP_BAD_REQUEST;
qd_compose_start_map(query->body);
qd_compose_end_map(query->body);
}

void qdra_link_update_CT(qdr_core_t *core,
qd_iterator_t *name,
qd_iterator_t *identity,
qdr_query_t *query,
qd_parsed_field_t *in_body)

{
// If the request was successful then the statusCode MUST contain 200 (OK) and the body of the message
// MUST contain a map containing the actual attributes of the entity updated. These MAY differ from those
// requested.
// A map containing attributes that are not applicable for the entity being created, or invalid values for a
// given attribute, MUST result in a failure response with a statusCode of 400 (Bad Request).
if (qd_parse_is_map(in_body)) {
// The absence of an attribute name implies that the entity should retain its existing value.
// If the map contains a key-value pair where the value is null then the updated entity should have no value
// for that attribute, removing any previous value.

qd_parsed_field_t *admin_state = qd_parse_value_by_key(in_body, qdr_link_columns[QDR_LINK_ADMIN_STATE]);
if (admin_state) { //admin state is the only field that can be updated via the update management request
//qd_iterator_t *adm_state = qd_parse_raw(admin_state);

if (identity) {
qdr_link_t *link = qdr_link_find_by_identity(core, identity);
// TODO - set the adm_state on the link
qdra_link_update_set_status(core, query, link);
}
else if (name) {
qdr_link_t *link = qdr_link_find_by_name(core, name);
// TODO - set the adm_state on the link
qdra_link_update_set_status(core, query, link);
}
else {
qdra_link_set_bad_request(query);
}
}
else
qdra_link_set_bad_request(query);

}
else
query->status = QD_AMQP_BAD_REQUEST;

//
// Enqueue the response.
//
qdr_agent_enqueue_response_CT(core, query);
}

2 changes: 0 additions & 2 deletions src/router_core/connections.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,6 @@ qdr_link_t *qdr_link_first_attach(qdr_connection_t *conn,
link->link_direction = dir;
link->capacity = conn->link_capacity;
link->credit_pending = conn->link_capacity;
link->admin_enabled = true;
link->oper_status = QDR_LINK_OPER_DOWN;
link->core_ticks = qdr_core_uptime_ticks(conn->core);
link->zero_credit_time = link->core_ticks;
Expand Down Expand Up @@ -1219,7 +1218,6 @@ qdr_link_t *qdr_create_link_CT(qdr_core_t *core,
link->disambiguated_name = 0;
link->terminus_addr = 0;
qdr_generate_link_name("qdlink", link->name, QD_DISCRIMINATOR_SIZE + 8);
link->admin_enabled = true;
link->oper_status = QDR_LINK_OPER_DOWN;
link->insert_prefix = 0;
link->strip_prefix = 0;
Expand Down
1 change: 0 additions & 1 deletion src/router_core/router_core_private.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,6 @@ struct qdr_link_t {
int credit_reported; ///< Number of credits to expose to management
uint32_t zero_credit_time; ///< Number of core ticks when credit last went to zero
bool reported_as_blocked; ///< The fact that this link has been blocked with zero credit has been logged
bool admin_enabled;
bool strip_annotations_in;
bool strip_annotations_out;
bool drain_mode;
Expand Down
2 changes: 1 addition & 1 deletion tests/system_tests_management.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ def test_get_operations(self):
result = self.node.get_operations()
for type in AMQP_LISTENER_TYPE, ROUTER_LINK_TYPE:
self.assertIn(type, result)
self.assertEqual(["UPDATE", "READ"], result[ROUTER_LINK_TYPE])
self.assertEqual(["READ"], result[ROUTER_LINK_TYPE])

def test_get_attributes(self):
result = self.node.get_attributes(type=DUMMY_TYPE)
Expand Down
44 changes: 44 additions & 0 deletions tests/system_tests_two_routers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,50 @@ def test_21_delete_connection_with_receiver(self):
self.assertEqual(test.error, None)
test.run()

def test_22_delete_update_inter_router_link(self):
"""
This test tries to delete an inter-router link but is
prevented from doing so.
"""
query_command = 'QUERY --type=link'
outputs = json.loads(self.run_skmanage(query_command))
passed = False

# Should fail trying to delete an inter-router link.
for output in outputs:
if "inter-router" == output['linkType']:
identity = output['identity']
if identity:
update_command = 'DELETE --type=link --id=' + identity
try:
json.loads(self.run_skmanage(update_command))
except Exception as e:
if "Forbidden" in str(e):
passed = True
break

# The test has passed since we were forbidden from deleting
# inter-router links.
self.assertTrue(passed)

passed = False
# Should fail trying to update the admin status an inter-router link.
for output in outputs:
if "inter-router" == output['linkType']:
identity = output['identity']
if identity:
update_command = 'UPDATE --type=link linkName=helloWorld --id=' + identity
try:
json.loads(self.run_skmanage(update_command))
except Exception as e:
if "Forbidden" in str(e):
passed = True
break

# The test has passed since we were forbidden from updating
# inter-router links.
self.assertTrue(passed)

def test_30_huge_address(self):
# try a link with an extremely long address
# DISPATCH-1461
Expand Down

0 comments on commit 745ca5b

Please sign in to comment.