Skip to content

Undeploy an API without having to delete the Host #1532

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

Merged
merged 14 commits into from
Apr 28, 2025
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
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
12 changes: 12 additions & 0 deletions golem-test-framework/src/components/worker_service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1247,6 +1247,18 @@ pub trait WorkerService: WorkerServiceInternal {
}

async fn kill(&self);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added undeploy to test-framework

async fn undeploy_api(&self, site: &str, id: &str, version: &str) -> crate::Result<()> {
match self.api_deployment_client() {
ApiDeploymentServiceClient::Grpc => not_available_on_grpc_api("undeploy_api"),
ApiDeploymentServiceClient::Http(client) => {
match client.undeploy_api(site, id, version).await {
Ok(_) => Ok(()),
Err(error) => Err(anyhow!("{error:?}")),
}
}
}
}
}

async fn new_worker_grpc_client(host: &str, grpc_port: u16) -> WorkerServiceGrpcClient<Channel> {
Expand Down
85 changes: 84 additions & 1 deletion golem-worker-service-base/src/service/gateway/api_deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use crate::gateway_api_definition::http::{
AllPathPatterns, CompiledAuthCallBackRoute, CompiledHttpApiDefinition, HttpApiDefinition, Route,
};

use crate::gateway_api_deployment::{ApiDeployment, ApiDeploymentRequest, ApiSite};
use crate::gateway_binding::GatewayBindingCompiled;
use crate::gateway_execution::router::{Router, RouterPattern};
use crate::repo::api_definition::ApiDefinitionRepo;
Expand All @@ -53,6 +54,7 @@ pub trait ApiDeploymentService<AuthCtx, Namespace> {
async fn undeploy(
&self,
deployment: &ApiDeploymentRequest<Namespace>,
auth_ctx: &AuthCtx,
) -> Result<(), ApiDeploymentError<Namespace>>;

// Example: A newer version of API definition is in dev site, and older version of the same definition-id is in prod site.
Expand Down Expand Up @@ -89,6 +91,14 @@ pub trait ApiDeploymentService<AuthCtx, Namespace> {
auth_ctx: &AuthCtx,
site: &ApiSiteString,
) -> Result<(), ApiDeploymentError<Namespace>>;

async fn undeploy_api(
&self,
namespace: &Namespace,
site: ApiSite,
api_definition_key: ApiDefinitionIdWithVersion,
auth_ctx: &AuthCtx,
) -> Result<(), ApiDeploymentError<Namespace>>;
}

#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -432,9 +442,11 @@ impl<Namespace: GolemNamespace, AuthCtx: GolemAuthCtx> ApiDeploymentService<Auth
async fn undeploy(
&self,
deployment: &ApiDeploymentRequest<Namespace>,
auth_ctx: &AuthCtx,
) -> Result<(), ApiDeploymentError<Namespace>> {
info!(namespace = %deployment.namespace, "Undeploying API definitions");
// Existing deployment

// 1. Get existing deployment records
let existing_deployment_records = self
.deployment_repo
.get_by_site(
Expand All @@ -445,6 +457,7 @@ impl<Namespace: GolemNamespace, AuthCtx: GolemAuthCtx> ApiDeploymentService<Auth

let mut remove_deployment_records: Vec<ApiDeploymentRecord> = vec![];

// 2. Filter records that match the API definition keys to undeploy
for deployment_record in existing_deployment_records {
if deployment_record.namespace != deployment.namespace.to_string()
|| deployment_record.subdomain != deployment.site.subdomain
Expand Down Expand Up @@ -473,13 +486,35 @@ impl<Namespace: GolemNamespace, AuthCtx: GolemAuthCtx> ApiDeploymentService<Auth
}

if !remove_deployment_records.is_empty() {
// 3. Get the specific API definitions being undeployed
Copy link
Contributor Author

@Nanashi-lab Nanashi-lab Apr 16, 2025

Choose a reason for hiding this comment

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

Removing component constraints during undeploy. There is already a test in service_test. Here we do two loops, one to filter by name, so there is no namespace conflict. And another loop to get the actual api definition, it is possible to do this in a single loop.

let mut definitions_to_undeploy = Vec::new();
for key in &deployment.api_definition_keys {
if let Some(definition) = self
.definition_repo
.get(&deployment.namespace.to_string(), &key.id.0, &key.version.0)
.await?
{
definitions_to_undeploy.push(
CompiledHttpApiDefinition::try_from(definition).map_err(|e| {
ApiDeploymentError::conversion_error("API definition record", e)
})?,
);
}
}

// 4. Remove component constraints
self.remove_component_constraints(definitions_to_undeploy, auth_ctx)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I hope you have a test around these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test for checking if component constraints have been removed, no not yet, I will add one

.await?;

// 5. Delete deployment records
self.deployment_repo
.delete(
&deployment.namespace.to_string(),
remove_deployment_records.clone(),
)
.await?;

// 6. Set undeployed as draft
self.set_undeployed_as_draft(remove_deployment_records)
.await?;
}
Expand Down Expand Up @@ -677,6 +712,54 @@ impl<Namespace: GolemNamespace, AuthCtx: GolemAuthCtx> ApiDeploymentService<Auth
Ok(())
}
}

async fn undeploy_api(
&self,
namespace: &Namespace,
site: ApiSite,
api_definition_key: ApiDefinitionIdWithVersion,
auth_ctx: &AuthCtx,
) -> Result<(), ApiDeploymentError<Namespace>> {
// Check if the site exists
let site_exists = self
.get_by_site(namespace, &ApiSiteString::from(&site))
.await?
.is_some();

if !site_exists {
return Err(ApiDeploymentError::ApiDeploymentNotFound(
namespace.clone(),
ApiSiteString::from(&site),
));
}

// Check if the API definition exists
let api_definition_exists = self
.get_by_id(namespace, Some(api_definition_key.id.clone()))
.await?
.iter()
.any(|deployment| {
deployment.api_definition_keys.iter().any(|key| {
key.id == api_definition_key.id && key.version == api_definition_key.version
})
});

if !api_definition_exists {
return Err(ApiDeploymentError::ApiDefinitionNotFound(
namespace.clone(),
api_definition_key.id,
api_definition_key.version,
));
}

let api_deployment = ApiDeploymentRequest {
namespace: namespace.clone(),
api_definition_keys: vec![api_definition_key],
site,
};

self.undeploy(&api_deployment, auth_ctx).await
}
}

// A structure representing the new deployments to be created
Expand Down
5 changes: 4 additions & 1 deletion golem-worker-service-base/tests/services_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,10 @@ async fn test_deployment(
));

let deployment = get_api_deployment("test.com", None, vec![&def3.id.0]);
deployment_service.undeploy(&deployment).await.unwrap();
deployment_service
.undeploy(&deployment, &EmptyAuthCtx::default())
.await
.unwrap();

let definitions: Vec<HttpApiDefinition> = deployment_service
.get_definitions_by_site(
Expand Down
75 changes: 74 additions & 1 deletion golem-worker-service/src/api/api_deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,18 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use golem_common::SafeDisplay;
use golem_common::{recorded_http_api_request, safe};
use golem_service_base::api_tags::ApiTags;
use golem_service_base::auth::{DefaultNamespace, EmptyAuthCtx};
use golem_worker_service_base::api::ApiEndpointError;
use golem_worker_service_base::api::{ApiDeployment, ApiDeploymentRequest};
use golem_worker_service_base::gateway_api_definition::ApiDefinitionId;
use golem_worker_service_base::gateway_api_definition::{ApiDefinitionId, ApiVersion};
use golem_worker_service_base::gateway_api_deployment;
use golem_worker_service_base::gateway_api_deployment::ApiSite;
use golem_worker_service_base::gateway_api_deployment::ApiSiteString;
use golem_worker_service_base::service::gateway::api_definition::ApiDefinitionIdWithVersion;
use golem_worker_service_base::service::gateway::api_deployment::ApiDeploymentError;
use golem_worker_service_base::service::gateway::api_deployment::ApiDeploymentService;
use poem_openapi::param::{Path, Query};
use poem_openapi::payload::Json;
Expand Down Expand Up @@ -182,4 +185,74 @@ impl ApiDeploymentApi {

Ok(Json("API deployment deleted".to_string()))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the endpoint. /:site/:id/:version

/// Undeploy a single API definition from a site
///
/// Removes a specific API definition (by id and version) from a site without deleting the entire deployment.
#[oai(
path = "/:site/:id/:version",
method = "delete",
operation_id = "undeploy_api"
)]
async fn undeploy_api(
&self,
site: Path<String>,
id: Path<String>,
version: Path<String>,
) -> Result<Json<String>, ApiEndpointError> {
let record = recorded_http_api_request!(
"undeploy_api",
site = site.0.clone(),
id = id.0.clone(),
version = version.0.clone()
);

let response = self
.undeploy_api_internal(site.0, id.0, version.0, &EmptyAuthCtx::default())
.instrument(record.span.clone())
.await;
record.result(response)
}

async fn undeploy_api_internal(
&self,
site: String,
id: String,
version: String,
auth_ctx: &EmptyAuthCtx,
) -> Result<Json<String>, ApiEndpointError> {
let namespace = DefaultNamespace::default();
let api_definition_key = ApiDefinitionIdWithVersion {
id: ApiDefinitionId(id),
version: ApiVersion(version),
};

// Split the site string into host and subdomain parts
let (host, subdomain) = if let Some(idx) = site.find('.') {
Copy link
Contributor

@afsalthaj afsalthaj Apr 23, 2025

Choose a reason for hiding this comment

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

This is unreliable. I don't think you can really split host and site like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fix this, there should be a way to convert ApiSiteString to ApiSite already within golem and use that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Finding host and subdomain by splitting dots is unreliable and may not work in all cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think there is a way to convert ApiSiteString to ApiSite. I think it's good to avoid that need, if you see what I mean.

let (subdomain, host) = site.split_at(idx);
(
host.trim_start_matches('.').to_string(),
Some(subdomain.to_string()),
)
} else {
(site, None)
};

let api_site = ApiSite { host, subdomain };
Copy link
Contributor

Choose a reason for hiding this comment

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

I think ApiSiteString may be of your help. may be your undeploy function can take this one instead of ApiSite.

That to undeploy an API from a site, you really don't need to logically split domain and subdomain anymore.


self.deployment_service
.undeploy_api(&namespace, api_site, api_definition_key, auth_ctx)
.await
.map_err(|err| match err {
ApiDeploymentError::ApiDeploymentNotFound(_, _) => {
ApiEndpointError::not_found(safe("Site not found".to_string()))
}
ApiDeploymentError::ApiDefinitionNotFound(_, _, _) => {
ApiEndpointError::not_found(safe("API definition not found".to_string()))
}
_ => ApiEndpointError::internal(safe(err.to_safe_string())),
})?;

Ok(Json("API definition undeployed from site".to_string()))
}
}
113 changes: 113 additions & 0 deletions integration-tests/tests/api/api_deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,3 +707,116 @@ async fn create_api_definition(
.await
.unwrap()
}

#[test]
#[tracing::instrument]
async fn undeploy_api_test(deps: &EnvBasedTestDependencies) {
let component_id = deps.component("shopping-cart").unique().store().await;

let api_definition_1 = create_api_definition(
deps,
&component_id,
Uuid::new_v4().to_string(),
"1".to_string(),
"/api/v1/path-1".to_string(),
)
.await;

let api_definition_2 = create_api_definition(
deps,
&component_id,
Uuid::new_v4().to_string(),
"2".to_string(),
"/api/v2/path-2".to_string(),
)
.await;

// Deploy both APIs to the same site
deps.worker_service()
.create_or_update_api_deployment(ApiDeploymentRequest {
api_definitions: vec![
ApiDefinitionInfo {
id: api_definition_1.id.as_ref().unwrap().value.clone(),
version: api_definition_1.version.clone(),
},
ApiDefinitionInfo {
id: api_definition_2.id.as_ref().unwrap().value.clone(),
version: api_definition_2.version.clone(),
},
],
site: ApiSite {
host: "localhost".to_string(),
subdomain: Some("undeploy-test".to_string()),
},
})
.await
.unwrap();

// List deployments and check both are present
let deployments = deps
.worker_service()
.list_api_deployments(None)
.await
.unwrap();
check!(deployments
.iter()
.any(|d| d.api_definitions.contains(&ApiDefinitionInfo {
id: api_definition_1.id.as_ref().unwrap().value.clone(),
version: api_definition_1.version.clone(),
})));
check!(deployments
.iter()
.any(|d| d.api_definitions.contains(&ApiDefinitionInfo {
id: api_definition_2.id.as_ref().unwrap().value.clone(),
version: api_definition_2.version.clone(),
})));

// Undeploy API 1
deps.worker_service()
.undeploy_api(
"undeploy-test.localhost",
&api_definition_1.id.as_ref().unwrap().value,
&api_definition_1.version,
)
.await
.unwrap();

// Verify that API 1 is no longer in the deployments
let deployments = deps
.worker_service()
.list_api_deployments(None)
.await
.unwrap();
check!(!deployments
.iter()
.any(|d| d.api_definitions.contains(&ApiDefinitionInfo {
id: api_definition_1.id.as_ref().unwrap().value.clone(),
version: api_definition_1.version.clone(),
})));

// Verify that API 2 is still in the deployments
check!(deployments
.iter()
.any(|d| d.api_definitions.contains(&ApiDefinitionInfo {
id: api_definition_2.id.as_ref().unwrap().value.clone(),
version: api_definition_2.version.clone(),
})));

// Test undeploying from a non-existent API
let result = deps
.worker_service()
.undeploy_api("subdomain.localhost", "non-existent-id", "1")
.await;
assert!(result.is_err());

// Test undeploying from a non-existent site
let result = deps
.worker_service()
.undeploy_api(
"non-existent.localhost",
&api_definition_2.id.as_ref().unwrap().value,
&api_definition_2.version,
)
.await;
assert!(result.is_err());
}
Loading
Loading