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.

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

Fixing API Gateway remaining tickets and bug fixes #543

Merged
merged 18 commits into from
Jun 6, 2024
Merged
10 changes: 7 additions & 3 deletions golem-cli/src/clients/api_deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use async_trait::async_trait;

use golem_client::model::{ApiDeployment, ApiSite};
use golem_client::model::{ApiDefinitionInfo, ApiDeployment, ApiSite};
use tracing::info;

use crate::model::{ApiDefinitionId, ApiDefinitionVersion, GolemError};
Expand Down Expand Up @@ -59,9 +59,13 @@ impl<C: golem_client::api::ApiDeploymentClient + Sync + Send> ApiDeploymentClien
.map_or("".to_string(), |s| format!("subdomain {}", s))
);

let deployment = ApiDeployment {
api_definition_id: api_definition_id.0.to_string(),
let api_definition = ApiDefinitionInfo {
id: api_definition_id.0.to_string(),
version: version.0.to_string(),
};

let deployment = ApiDeployment {
api_definitions: vec![api_definition],
site: ApiSite {
host: host.to_string(),
subdomain,
Expand Down
53 changes: 28 additions & 25 deletions golem-cli/src/model/text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,17 +408,19 @@ impl TextFormat for ScanCursor {

impl TextFormat for ApiDeployment {
fn print(&self) {
printdoc!(
"
API deployment on {} with definition {}/{}
",
match &self.site.subdomain {
Some(subdomain) => format!("{}.{}", subdomain, self.site.host),
None => self.site.host.to_string(),
},
self.api_definition_id,
self.version,
);
for api_defs in &self.api_definitions {
printdoc!(
"
API deployment on {} with definition {}/{}
",
match &self.site.subdomain {
Some(subdomain) => format!("{}.{}", subdomain, self.site.host),
None => self.site.host.to_string(),
},
api_defs.id,
api_defs.version,
);
}
}
}

Expand All @@ -432,24 +434,25 @@ struct ApiDeploymentView {
pub version: String,
}

impl From<&ApiDeployment> for ApiDeploymentView {
fn from(value: &ApiDeployment) -> Self {
ApiDeploymentView {
site: match &value.site.subdomain {
Some(subdomain) => format!("{}.{}", subdomain, value.site.host),
None => value.site.host.to_string(),
},
id: value.api_definition_id.to_string(),
version: value.version.to_string(),
}
}
}

impl TextFormat for Vec<ApiDeployment> {
fn print(&self) {
print_stdout(
self.iter()
.map(ApiDeploymentView::from)
.flat_map(|deployment| {
deployment
.api_definitions
.iter()
.map(|def| ApiDeploymentView {
site: match &deployment.site.subdomain {
Some(subdomain) => {
format!("{}.{}", subdomain, deployment.site.host)
}
None => deployment.site.host.to_string(),
},
id: def.id.to_string(),
version: def.version.to_string(),
})
})
.collect::<Vec<_>>()
.with_title(),
)
Expand Down
10 changes: 6 additions & 4 deletions golem-cli/tests/api_deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,20 @@ fn api_deployment_deploy(
"sdomain",
])?;

let api_definition_info = deployment.api_definitions.first().unwrap();

assert_eq!(deployment.site.subdomain, Some("sdomain".to_string()));
assert_eq!(deployment.site.host, host);
assert_eq!(deployment.api_definition_id, definition.id);
assert_eq!(deployment.version, definition.version);
assert_eq!(api_definition_info.id, definition.id);
assert_eq!(api_definition_info.version, definition.version);

let updated_def: HttpApiDefinition = cli.run(&[
"api-definition",
"get",
&cfg.arg('i', "id"),
&deployment.api_definition_id,
&deployment.api_definitions.first().unwrap().id,
&cfg.arg('V', "version"),
&deployment.version,
&deployment.api_definitions.first().unwrap().version,
])?;

assert!(definition.draft);
Expand Down
6 changes: 6 additions & 0 deletions golem-worker-service-base/src/api/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,12 @@ mod conversion {
ApiDeploymentError::DeploymentConflict(conflict) => {
ApiEndpointError::already_exists(format!("Deployment conflict: {}", conflict))
}
ApiDeploymentError::ConflictingDefinitions(conflicts) => {
ApiEndpointError::already_exists(format!(
"Conflicting API definitions during deployment: {}",
conflicts.join(", ")
))
}
}
}
}
Expand Down
15 changes: 6 additions & 9 deletions golem-worker-service-base/src/api/custom_http_request_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use poem::{Body, Endpoint, Request, Response};
use tracing::{error, info};

use crate::http::{ApiInputPath, InputHttpRequest};
use crate::service::api_definition_lookup::ApiDefinitionLookup;
use crate::service::api_definition_lookup::ApiDefinitionsLookup;

use crate::worker_binding::WorkerBindingResolver;
use crate::worker_bridge_execution::WorkerRequestExecutor;
Expand All @@ -21,15 +21,15 @@ pub struct CustomHttpRequestApi {
pub evaluator: Arc<dyn Evaluator + Sync + Send>,
pub worker_metadata_fetcher: Arc<dyn WorkerMetadataFetcher + Sync + Send>,
pub api_definition_lookup_service:
Arc<dyn ApiDefinitionLookup<InputHttpRequest, HttpApiDefinition> + Sync + Send>,
Arc<dyn ApiDefinitionsLookup<InputHttpRequest, HttpApiDefinition> + Sync + Send>,
}

impl CustomHttpRequestApi {
pub fn new(
worker_request_executor_service: Arc<dyn WorkerRequestExecutor + Sync + Send>,
worker_metadata_fetcher: Arc<dyn WorkerMetadataFetcher + Sync + Send>,
api_definition_lookup_service: Arc<
dyn ApiDefinitionLookup<InputHttpRequest, HttpApiDefinition> + Sync + Send,
dyn ApiDefinitionsLookup<InputHttpRequest, HttpApiDefinition> + Sync + Send,
>,
) -> Self {
let evaluator = Arc::new(DefaultEvaluator::from_worker_request_executor(
Expand Down Expand Up @@ -83,7 +83,7 @@ impl CustomHttpRequestApi {
req_body: json_request_body,
};

let api_definition = match self
let possible_api_definitions = match self
.api_definition_lookup_service
.get(api_request.clone())
.await
Expand All @@ -97,18 +97,15 @@ impl CustomHttpRequestApi {
}
};

match api_request.resolve(&api_definition).await {
match api_request.resolve(possible_api_definitions).await {
Ok(resolved_worker_request) => {
resolved_worker_request
.execute_with::<poem::Response>(&self.evaluator, &self.worker_metadata_fetcher)
.await
}

Err(msg) => {
error!(
"API request id: {} - request error: {}",
&api_definition.id, msg
);
error!("Failed to resolve the API definition; error: {}", msg);

Response::builder()
.status(StatusCode::METHOD_NOT_ALLOWED)
Expand Down
23 changes: 19 additions & 4 deletions golem-worker-service-base/src/api/register_api_definition_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,18 @@ use crate::parser::ParseError;
#[serde(rename_all = "camelCase")]
#[oai(rename_all = "camelCase")]
pub struct ApiDeployment {
pub api_definition_id: ApiDefinitionId,
pub version: ApiVersion,
pub api_definitions: Vec<ApiDefinitionInfo>,
pub site: ApiSite,
}

#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Object)]
#[serde(rename_all = "camelCase")]
#[oai(rename_all = "camelCase")]
pub struct ApiDefinitionInfo {
pub id: ApiDefinitionId,
pub version: ApiVersion,
}

// Mostly this data structures that represents the actual incoming request
// exist due to the presence of complicated Expr data type in api_definition::ApiDefinition.
// Consider them to be otherwise same
Expand Down Expand Up @@ -54,9 +61,17 @@ pub struct GolemWorkerBinding {

impl<N> From<crate::api_definition::ApiDeployment<N>> for ApiDeployment {
fn from(value: crate::api_definition::ApiDeployment<N>) -> Self {
let api_definitions = value
.api_definition_keys
.into_iter()
.map(|key| ApiDefinitionInfo {
id: key.id,
version: key.version,
})
.collect();

Self {
api_definition_id: value.api_definition_id.id,
version: value.api_definition_id.version,
api_definitions,
site: value.site,
}
}
Expand Down
5 changes: 3 additions & 2 deletions golem-worker-service-base/src/api_definition/api_common.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::fmt::Debug;
use std::fmt::Display;

use crate::service::api_definition::ApiDefinitionKey;
use crate::service::api_definition::ApiDefinitionIdWithVersion;
use bincode::{Decode, Encode};
use poem_openapi::NewType;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -51,7 +51,8 @@ pub trait HasGolemWorkerBindings {
Eq, Hash, PartialEq, Clone, Debug, serde::Deserialize, bincode::Encode, bincode::Decode,
)]
pub struct ApiDeployment<Namespace> {
pub api_definition_id: ApiDefinitionKey<Namespace>,
pub namespace: Namespace,
pub api_definition_keys: Vec<ApiDefinitionIdWithVersion>,
pub site: ApiSite,
}

Expand Down
Loading
Loading