Back out "silo-scoped endpoint to list project-scoped schemas"

david-crespo · david-crespo · commit 6d77cd9c469a · 2024-11-13T17:03:09.000-06:00
This backs out commit 7f38e7b.
diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt
@@ -74,7 +74,6 @@ API operations found with tag "metrics"
 OPERATION ID                             METHOD   URL PATH
 silo_metric                              GET      /v1/metrics/{metric_name}
 timeseries_query                         POST     /v1/timeseries/query
-timeseries_schema_list                   GET      /v1/timeseries/schema
 
 API operations found with tag "policy"
 OPERATION ID                             METHOD   URL PATH
diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs
@@ -2567,22 +2567,6 @@ pub trait NexusExternalApi {
         body: TypedBody<params::TimeseriesQuery>,
     ) -> Result<HttpResponseOk<views::OxqlQueryResult>, HttpError>;
 
-    /// List project-scoped timeseries schemas
-    ///
-    /// List schemas that can be queried through the `/v1/timeseries/query` endpoint.
-    #[endpoint {
-        method = GET,
-        path = "/v1/timeseries/schema",
-        tags = ["metrics"],
-    }]
-    async fn timeseries_schema_list(
-        rqctx: RequestContext<Self::Context>,
-        pag_params: Query<TimeseriesSchemaPaginationParams>,
-    ) -> Result<
-        HttpResponseOk<ResultsPage<oximeter_types::TimeseriesSchema>>,
-        HttpError,
-    >;
-
     /// Run project-scoped timeseries query
     ///
     /// Queries are written in OxQL. Project must be specified by name or ID in
diff --git a/nexus/src/app/metrics.rs b/nexus/src/app/metrics.rs
@@ -14,7 +14,6 @@ use nexus_db_queries::{
 use nexus_external_api::TimeseriesSchemaPaginationParams;
 use nexus_types::external_api::params::SystemMetricName;
 use omicron_common::api::external::{Error, InternalContext};
-use oximeter::AuthzScope;
 use oximeter_db::{Measurement, TimeseriesSchema};
 use std::num::NonZeroU32;
 
@@ -167,7 +166,7 @@ impl super::Nexus {
     }
 
     /// Run an OxQL query against the timeseries database, scoped to a specific project.
-    pub(crate) async fn project_timeseries_query(
+    pub(crate) async fn timeseries_query_project(
         &self,
         _opctx: &OpContext,
         project_lookup: &lookup::Project<'_>,
@@ -202,45 +201,4 @@ impl super::Nexus {
                 _ => Error::InternalError { internal_message: e.to_string() },
             })
     }
-
-    /// List available project-scoped timeseries schema
-    pub(crate) async fn project_timeseries_schema_list(
-        &self,
-        opctx: &OpContext,
-        pagination: &TimeseriesSchemaPaginationParams,
-        limit: NonZeroU32,
-    ) -> Result<dropshot::ResultsPage<TimeseriesSchema>, Error> {
-        // any authenticated user should be able to do this
-        let authz_silo = opctx
-            .authn
-            .silo_required()
-            .internal_context("listing project-scoped timeseries schemas")?;
-        opctx.authorize(authz::Action::ListChildren, &authz_silo).await?;
-
-        self.timeseries_client
-            .timeseries_schema_list(&pagination.page, limit)
-            .await
-            .and_then(|schemas| {
-                let filtered = schemas
-                    .items
-                    .into_iter()
-                    .filter(|schema| schema.authz_scope == AuthzScope::Project)
-                    .collect();
-                dropshot::ResultsPage::new(
-                    filtered,
-                    &dropshot::EmptyScanParams {},
-                    |schema, _| schema.timeseries_name.clone(),
-                )
-                .map_err(|err| oximeter_db::Error::Database(err.to_string()))
-            })
-            .map_err(|e| match e {
-                oximeter_db::Error::DatabaseUnavailable(_)
-                | oximeter_db::Error::Connection(_) => {
-                    Error::ServiceUnavailable {
-                        internal_message: e.to_string(),
-                    }
-                }
-                _ => Error::InternalError { internal_message: e.to_string() },
-            })
-    }
 }
diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs
@@ -5544,33 +5544,6 @@ impl NexusExternalApi for NexusExternalApiImpl {
             .await
     }
 
-    async fn timeseries_schema_list(
-        rqctx: RequestContext<ApiContext>,
-        pag_params: Query<TimeseriesSchemaPaginationParams>,
-    ) -> Result<
-        HttpResponseOk<ResultsPage<oximeter_db::TimeseriesSchema>>,
-        HttpError,
-    > {
-        let apictx = rqctx.context();
-        let handler = async {
-            let nexus = &apictx.context.nexus;
-            let opctx =
-                crate::context::op_context_for_external_api(&rqctx).await?;
-            let pagination = pag_params.into_inner();
-            let limit = rqctx.page_limit(&pagination)?;
-            nexus
-                .project_timeseries_schema_list(&opctx, &pagination, limit)
-                .await
-                .map(HttpResponseOk)
-                .map_err(HttpError::from)
-        };
-        apictx
-            .context
-            .external_latencies
-            .instrument_dropshot_handler(&rqctx, handler)
-            .await
-    }
-
     async fn timeseries_query(
         rqctx: RequestContext<ApiContext>,
         query_params: Query<params::ProjectSelector>,
@@ -5586,7 +5559,7 @@ impl NexusExternalApi for NexusExternalApiImpl {
             let project_lookup =
                 nexus.project_lookup(&opctx, project_selector)?;
             nexus
-                .project_timeseries_query(&opctx, &project_lookup, &query)
+                .timeseries_query_project(&opctx, &project_lookup, &query)
                 .await
                 .map(|tables| HttpResponseOk(views::OxqlQueryResult { tables }))
                 .map_err(HttpError::from)
diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs
@@ -949,8 +949,6 @@ pub static DEMO_SILO_METRICS_URL: Lazy<String> = Lazy::new(|| {
 pub static TIMESERIES_QUERY_URL: Lazy<String> = Lazy::new(|| {
     format!("/v1/timeseries/query?project={}", *DEMO_PROJECT_NAME)
 });
-pub static TIMESERIES_LIST_URL: Lazy<String> =
-    Lazy::new(|| String::from("/v1/timeseries/schema"));
 
 pub static SYSTEM_TIMESERIES_LIST_URL: Lazy<String> =
     Lazy::new(|| String::from("/v1/system/timeseries/schemas"));
@@ -2211,15 +2209,6 @@ pub static VERIFY_ENDPOINTS: Lazy<Vec<VerifyEndpoint>> = Lazy::new(|| {
             ],
         },
 
-        VerifyEndpoint {
-            url: &TIMESERIES_LIST_URL,
-            visibility: Visibility::Public,
-            unprivileged_access: UnprivilegedAccess::None,
-            allowed_methods: vec![
-                AllowedMethod::GetVolatile,
-            ],
-        },
-
         VerifyEndpoint {
             url: &TIMESERIES_QUERY_URL,
             visibility: Visibility::Protected,
diff --git a/nexus/tests/integration_tests/metrics.rs b/nexus/tests/integration_tests/metrics.rs
@@ -259,33 +259,6 @@ async fn test_metrics(
     assert_silo_metrics(&cptestctx, Some(project1_id), GIB, 4, GIB).await;
 }
 
-/// Test that we can correctly list some timeseries schema.
-#[nexus_test]
-async fn test_timeseries_schema_list(
-    cptestctx: &ControlPlaneTestContext<omicron_nexus::Server>,
-) {
-    // Nexus registers itself as a metric producer on startup, with its own UUID
-    // as the producer ID. Wait for this to show up in the registered lists of
-    // producers.
-    let nexus_id = cptestctx.server.server_context().nexus.id();
-    wait_for_producer(&cptestctx.oximeter, nexus_id).await;
-
-    // We should be able to fetch the list of timeseries, and it should include
-    // Nexus's HTTP latency distribution. This is defined in Nexus itself, and
-    // should always exist after we've registered as a producer and start
-    // producing data. Force a collection to ensure that happens.
-    cptestctx.oximeter.force_collect().await;
-    let client = &cptestctx.external_client;
-    let url = "/v1/timeseries/schema";
-    let schema =
-        objects_list_page_authz::<TimeseriesSchema>(client, &url).await;
-    // request latency metric that shows up in the system endpoint is filtered out here
-    assert!(schema.items.is_empty());
-
-    // TODO: add a project-scoped metric and fetch again
-    // TODO: I think even unprivileged user should be able to list these
-}
-
 /// Test that we can correctly list some timeseries schema.
 #[nexus_test]
 async fn test_system_timeseries_schema_list(
diff --git a/openapi/nexus.json b/openapi/nexus.json
@@ -8939,59 +8939,6 @@
         }
       }
     },
-    "/v1/timeseries/schema": {
-      "get": {
-        "tags": [
-          "metrics"
-        ],
-        "summary": "List project-scoped timeseries schemas",
-        "description": "List schemas that can be queried through the `/v1/timeseries/query` endpoint.",
-        "operationId": "timeseries_schema_list",
-        "parameters": [
-          {
-            "in": "query",
-            "name": "limit",
-            "description": "Maximum number of items returned by a single call",
-            "schema": {
-              "nullable": true,
-              "type": "integer",
-              "format": "uint32",
-              "minimum": 1
-            }
-          },
-          {
-            "in": "query",
-            "name": "page_token",
-            "description": "Token returned by previous call to retrieve the subsequent page",
-            "schema": {
-              "nullable": true,
-              "type": "string"
-            }
-          }
-        ],
-        "responses": {
-          "200": {
-            "description": "successful operation",
-            "content": {
-              "application/json": {
-                "schema": {
-                  "$ref": "#/components/schemas/TimeseriesSchemaResultsPage"
-                }
-              }
-            }
-          },
-          "4XX": {
-            "$ref": "#/components/responses/Error"
-          },
-          "5XX": {
-            "$ref": "#/components/responses/Error"
-          }
-        },
-        "x-dropshot-pagination": {
-          "required": []
-        }
-      }
-    },
     "/v1/users": {
       "get": {
         "tags": [
