Skip to content

Commit

Permalink
[RHCLOUD-32946] Fix slack integration creation issue (RedHatInsights#…
Browse files Browse the repository at this point in the history 
…2728)
  • Loading branch information
@g-duval
g-duval authored May 29, 2024
1 parent 95357c9 commit a242e1b
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 107 deletions.
4 changes: 0 additions & 4 deletions .rhcicd/clowdapp-backend.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,6 @@ objects:
value: ${NOTIFICATIONS_USE_DEFAULT_TEMPLATE}
- name: NOTIFICATIONS_DRAWER_ENABLED
value: ${NOTIFICATIONS_DRAWER_ENABLED}
- name: NOTIFICATIONS_SLACK_FORBID_CHANNEL_USAGE_ENABLED
value: ${NOTIFICATIONS_SLACK_FORBID_CHANNEL_USAGE_ENABLED}
- name: SOURCES_PSK
valueFrom:
secretKeyRef:
Expand Down Expand Up @@ -260,5 +258,3 @@ parameters:
value: "false"
- name: NOTIFICATIONS_DRAWER_ENABLED
value: "false"
- name: NOTIFICATIONS_SLACK_FORBID_CHANNEL_USAGE_ENABLED
value: "false"
15 changes: 0 additions & 15 deletions backend/src/main/java/com/redhat/cloud/notifications/config/BackendConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ public class BackendConfig {
* Unleash configuration
*/
private String drawerToggle;
private String forbidSlackChannelUsageToggle;
private String uniqueBgNameToggle;
private String uniqueIntegrationNameToggle;

Expand All @@ -52,10 +51,6 @@ private static String toggleName(String feature) {
@Deprecated(forRemoval = true, since = "To be removed when we're done migrating to Unleash in all environments")
boolean drawerEnabled;

@ConfigProperty(name = "notifications.slack.forbid.channel.usage.enabled", defaultValue = "false")
@Deprecated(forRemoval = true, since = "To be removed when we're done migrating to Unleash in all environments")
boolean slackForbidChannelUsageEnabled;

// Only used in stage environments.
@ConfigProperty(name = DEFAULT_TEMPLATE, defaultValue = "false")
boolean defaultTemplateEnabled;
Expand All @@ -77,7 +72,6 @@ private static String toggleName(String feature) {
@PostConstruct
void postConstruct() {
drawerToggle = toggleRegistry.register("drawer", true);
forbidSlackChannelUsageToggle = toggleRegistry.register("forbid-slack-channel-usage", true);
uniqueBgNameToggle = toggleRegistry.register("unique-bg-name", true);
uniqueIntegrationNameToggle = toggleRegistry.register("unique-integration-name", true);
}
Expand All @@ -89,7 +83,6 @@ void logConfigAtStartup(@Observes Startup event) {
config.put(drawerToggle, isDrawerEnabled());
config.put(EMAILS_ONLY_MODE, isEmailsOnlyModeEnabled());
config.put(INSTANT_EMAILS, isInstantEmailsEnabled());
config.put(forbidSlackChannelUsageToggle, isForbidSlackChannelUsage());
config.put(uniqueBgNameToggle, isUniqueBgNameEnabled());
config.put(uniqueIntegrationNameToggle, isUniqueIntegrationNameEnabled());
config.put(UNLEASH, unleashEnabled);
Expand All @@ -116,14 +109,6 @@ public boolean isEmailsOnlyModeEnabled() {
return emailsOnlyModeEnabled;
}

public boolean isForbidSlackChannelUsage() {
if (unleashEnabled) {
return unleash.isEnabled(forbidSlackChannelUsageToggle, false);
} else {
return slackForbidChannelUsageEnabled;
}
}

public boolean isInstantEmailsEnabled() {
return instantEmailsEnabled;
}
Expand Down
23 changes: 8 additions & 15 deletions backend/src/main/java/com/redhat/cloud/notifications/routers/EndpointResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@
// TODO Needs documentation annotations
public class EndpointResource {

public static final String EMPTY_SLACK_CHANNEL_ERROR = "The channel field is required";
public static final String DEPRECATED_SLACK_CHANNEL_ERROR = "The channel field is deprecated";
public static final String UNSUPPORTED_ENDPOINT_TYPE = "Unsupported endpoint type";
public static final String REDACTED_CREDENTIAL = "*****";
Expand Down Expand Up @@ -327,20 +326,14 @@ public Endpoint internalCreateEndpoint(
}

private void checkSlackChannel(CamelProperties camelProperties, CamelProperties previousCamelProperties) {
String channel = camelProperties.getExtras().get("channel");

if (backendConfig.isForbidSlackChannelUsage()) {
// throw an exception if we receive a channel on endpoint creation
if (null == previousCamelProperties && channel != null) {
throw new BadRequestException(DEPRECATED_SLACK_CHANNEL_ERROR);
// throw an exception if we receive a channel update
} else if (channel != null && !channel.equals(previousCamelProperties.getExtras().get("channel"))) {
throw new BadRequestException(DEPRECATED_SLACK_CHANNEL_ERROR);
}
} else {
if (channel == null || channel.isBlank()) {
throw new BadRequestException(EMPTY_SLACK_CHANNEL_ERROR);
}
String channel = camelProperties.getExtras() != null ? camelProperties.getExtras().get("channel") : null;

// throw an exception if we receive a channel on endpoint creation
if (null == previousCamelProperties && channel != null) {
throw new BadRequestException(DEPRECATED_SLACK_CHANNEL_ERROR);
// throw an exception if we receive a channel update
} else if (channel != null && !channel.equals(previousCamelProperties.getExtras().get("channel"))) {
throw new BadRequestException(DEPRECATED_SLACK_CHANNEL_ERROR);
}
}

Expand Down
91 changes: 18 additions & 73 deletions backend/src/test/java/com/redhat/cloud/notifications/routers/EndpointResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@
import static com.redhat.cloud.notifications.models.EndpointType.WEBHOOK;
import static com.redhat.cloud.notifications.models.HttpType.POST;
import static com.redhat.cloud.notifications.routers.EndpointResource.DEPRECATED_SLACK_CHANNEL_ERROR;
import static com.redhat.cloud.notifications.routers.EndpointResource.EMPTY_SLACK_CHANNEL_ERROR;
import static io.restassured.RestAssured.given;
import static io.restassured.http.ContentType.JSON;
import static io.restassured.http.ContentType.TEXT;
Expand All @@ -84,6 +83,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
Expand Down Expand Up @@ -635,68 +635,8 @@ void addBogusCamelEndpoint() {

}

@Test
void testMissingSlackChannel() {
String identityHeaderValue = TestHelpers.encodeRHIdentityInfo("account-id", "org-id", "user");
Header identityHeader = TestHelpers.createRHIdentityHeader(identityHeaderValue);
MockServerConfig.addMockRbacAccess(identityHeaderValue, FULL_ACCESS);

Map<String, String> extras = new HashMap<>(Map.of("channel", "")); // An empty channel value is invalid.
CamelProperties camelProperties = new CamelProperties();
camelProperties.setUrl("https://foo.com");
camelProperties.setExtras(extras);

Endpoint endpoint = new Endpoint();
endpoint.setType(EndpointType.CAMEL);
endpoint.setSubType("slack");
endpoint.setName("name");
endpoint.setDescription("description");
endpoint.setProperties(camelProperties);

String responseBody = given()
.header(identityHeader)
.when()
.contentType(JSON)
.body(Json.encode(this.endpointMapper.toDTO(endpoint)))
.post("/endpoints")
.then()
.statusCode(400)
.extract().asString();

assertEquals(EMPTY_SLACK_CHANNEL_ERROR, responseBody);

extras.put("channel", " "); // A blank channel value is invalid.

responseBody = given()
.header(identityHeader)
.when()
.contentType(JSON)
.body(Json.encode(this.endpointMapper.toDTO(endpoint)))
.post("/endpoints")
.then()
.statusCode(400)
.extract().asString();

assertEquals(EMPTY_SLACK_CHANNEL_ERROR, responseBody);

extras.remove("channel"); // A missing channel value is invalid.

responseBody = given()
.header(identityHeader)
.when()
.contentType(JSON)
.body(Json.encode(this.endpointMapper.toDTO(endpoint)))
.post("/endpoints")
.then()
.statusCode(400)
.extract().asString();

assertEquals(EMPTY_SLACK_CHANNEL_ERROR, responseBody);
}

@Test
void testForbidSlackChannelUsage() {
when(backendConfig.isForbidSlackChannelUsage()).thenReturn(true);

String identityHeaderValue = TestHelpers.encodeRHIdentityInfo("account-id", "org-id", "user");
Header identityHeader = TestHelpers.createRHIdentityHeader(identityHeaderValue);
Expand Down Expand Up @@ -763,6 +703,19 @@ void testForbidSlackChannelUsage() {
.put("/endpoints/{id}")
.then()
.statusCode(400);

// test create slack integration without extras object
camelProperties.setExtras(null);
endpoint.setProperties(camelProperties);
given()
.header(identityHeader)
.when()
.contentType(JSON)
.body(Json.encode(this.endpointMapper.toDTO(endpoint)))
.post("/endpoints")
.then()
.statusCode(200)
.extract().asString();
}

@Test
Expand All @@ -779,9 +732,6 @@ void addSlackEndpoint() {
CamelProperties cAttr = new CamelProperties();
cAttr.setDisableSslVerification(false);
cAttr.setUrl(getMockServerUrl());
Map<String, String> extras = new HashMap<>();
extras.put("channel", "#notifications");
cAttr.setExtras(extras);

Endpoint ep = new Endpoint();
ep.setType(EndpointType.CAMEL);
Expand Down Expand Up @@ -816,11 +766,8 @@ void addSlackEndpoint() {
assertTrue(endpoint.getBoolean("enabled"));
assertEquals("slack", endpoint.getString("sub_type"));
JsonObject extrasObject = properties.getJsonObject("extras");
assertNotNull(extrasObject);
String channel = extrasObject.getString("channel");
assertEquals("#notifications", channel);
assertNull(extrasObject);

ep.getProperties(CamelProperties.class).getExtras().put("channel", "#updated");
ep.getProperties(CamelProperties.class).setUrl("https://redhat.com");

// Now update
Expand All @@ -839,7 +786,6 @@ void addSlackEndpoint() {
CamelProperties updatedProperties = entityManager.createQuery("FROM CamelProperties WHERE id = :id", CamelProperties.class)
.setParameter("id", UUID.fromString(id))
.getSingleResult();
assertEquals("#updated", updatedProperties.getExtras().get("channel"));
assertEquals(ep.getProperties(CamelProperties.class).getUrl(), updatedProperties.getUrl());

} finally {
Expand Down Expand Up @@ -2179,7 +2125,6 @@ void testEndpointValidUrls() {
for (final var url : ValidNonPrivateUrlValidatorTest.validUrls) {
// Test with a camel endpoint.
camelProperties.setUrl(url);
camelProperties.setExtras(Map.of("channel", "notifications"));
endpoint.setType(EndpointType.CAMEL);
endpoint.setProperties(camelProperties);
endpoint.setSubType(subType);
Expand Down Expand Up @@ -2311,7 +2256,7 @@ void testEndpointTest() {

Assertions.assertEquals(createdEndpoint.getId(), sentPayload.endpointUuid, "the sent endpoint UUID in the payload doesn't match the one from the fixture");
Assertions.assertEquals(orgId, sentPayload.orgId, "the sent org id in the payload doesn't match the one from the fixture");
Assertions.assertNull(sentPayload.message, "the sent message should be null since no custom message was specified");
assertNull(sentPayload.message, "the sent message should be null since no custom message was specified");
}

/**
Expand Down Expand Up @@ -2626,8 +2571,8 @@ public void testUpdateEndpointDeleteSecrets() {
final Endpoint databaseEndpoint = this.endpointRepository.getEndpoint(DEFAULT_ORG_ID, endpointUuid);
final WebhookProperties webhookProperties = databaseEndpoint.getProperties(WebhookProperties.class);

Assertions.assertNull(webhookProperties.getBasicAuthenticationSourcesId(), "Sources was called to delete the basic authentication secret, but the secret's ID wasn't deleted from the database");
Assertions.assertNull(webhookProperties.getSecretTokenSourcesId(), "Sources was called to delete the secret token secret, but the secret's ID wasn't deleted from the database");
assertNull(webhookProperties.getBasicAuthenticationSourcesId(), "Sources was called to delete the basic authentication secret, but the secret's ID wasn't deleted from the database");
assertNull(webhookProperties.getSecretTokenSourcesId(), "Sources was called to delete the secret token secret, but the secret's ID wasn't deleted from the database");
}

/**
Expand Down

0 comments on commit a242e1b

Please sign in to comment.