fix: removing duplicate creation of user if user does not exist already (#9455)

* fix: removing duplicate creation of user if user does not exist already

* test: adding api test case

* fix: updated test cases

* fix: remove unnecessary logging and clean up connection params API

* feat: add gateway fixture and integrate with signoz for connection parameters

* feat: add cloudintegrations to the test job matrix in integrationci.yaml

* fix: remove outdated comments from make_http_mocks fixture

* fix: remove deprecated ZeusURL from build configurations
This commit is contained in:
swapnil-signoz
2025-11-03 16:45:08 +05:30
committed by GitHub
parent 105c3a3b8c
commit bbf987ebd7
12 changed files with 251 additions and 50 deletions

View File

@@ -107,7 +107,6 @@ jobs:
-X github.com/SigNoz/signoz/pkg/version.branch=${{ needs.prepare.outputs.branch }}
-X github.com/SigNoz/signoz/ee/zeus.url=https://api.signoz.cloud
-X github.com/SigNoz/signoz/ee/zeus.deprecatedURL=https://license.signoz.io
-X github.com/SigNoz/signoz/ee/query-service/constants.ZeusURL=https://api.signoz.cloud
-X github.com/SigNoz/signoz/ee/query-service/constants.LicenseSignozIo=https://license.signoz.io/api/v1
-X github.com/SigNoz/signoz/pkg/analytics.key=9kRrJ7oPCGPEJLF6QjMPLt5bljFhRQBr'
DOCKER_BASE_IMAGES: '{"alpine": "alpine:3.20.3"}'

View File

@@ -106,7 +106,6 @@ jobs:
-X github.com/SigNoz/signoz/pkg/version.branch=${{ needs.prepare.outputs.branch }}
-X github.com/SigNoz/signoz/ee/zeus.url=https://api.staging.signoz.cloud
-X github.com/SigNoz/signoz/ee/zeus.deprecatedURL=https://license.staging.signoz.cloud
-X github.com/SigNoz/signoz/ee/query-service/constants.ZeusURL=https://api.staging.signoz.cloud
-X github.com/SigNoz/signoz/ee/query-service/constants.LicenseSignozIo=https://license.staging.signoz.cloud/api/v1
-X github.com/SigNoz/signoz/pkg/analytics.key=9kRrJ7oPCGPEJLF6QjMPLt5bljFhRQBr'
DOCKER_BASE_IMAGES: '{"alpine": "alpine:3.20.3"}'

View File

@@ -17,6 +17,7 @@ jobs:
- bootstrap
- passwordauthn
- callbackauthn
- cloudintegrations
- querier
- ttl
sqlstore-provider:

View File

@@ -31,7 +31,6 @@ builds:
- -X github.com/SigNoz/signoz/pkg/version.branch={{ .Branch }}
- -X github.com/SigNoz/signoz/ee/zeus.url=https://api.signoz.cloud
- -X github.com/SigNoz/signoz/ee/zeus.deprecatedURL=https://license.signoz.io
- -X github.com/SigNoz/signoz/ee/query-service/constants.ZeusURL=https://api.signoz.cloud
- -X github.com/SigNoz/signoz/ee/query-service/constants.LicenseSignozIo=https://license.signoz.io/api/v1
- -X github.com/SigNoz/signoz/pkg/analytics.key=9kRrJ7oPCGPEJLF6QjMPLt5bljFhRQBr
mod_timestamp: "{{ .CommitTimestamp }}"

View File

@@ -10,7 +10,6 @@ import (
"strings"
"time"
"github.com/SigNoz/signoz/ee/query-service/constants"
"github.com/SigNoz/signoz/pkg/errors"
"github.com/SigNoz/signoz/pkg/http/render"
"github.com/SigNoz/signoz/pkg/modules/user"
@@ -77,7 +76,7 @@ func (ah *APIHandler) CloudIntegrationsGenerateConnectionParams(w http.ResponseW
return
}
ingestionUrl, signozApiUrl, apiErr := getIngestionUrlAndSigNozAPIUrl(r.Context(), license.Key)
ingestionUrl, signozApiUrl, apiErr := ah.getIngestionUrlAndSigNozAPIUrl(r.Context(), license.Key)
if apiErr != nil {
RespondError(w, basemodel.WrapApiError(
apiErr, "couldn't deduce ingestion url and signoz api url",
@@ -186,48 +185,37 @@ func (ah *APIHandler) getOrCreateCloudIntegrationUser(
return cloudIntegrationUser, nil
}
func getIngestionUrlAndSigNozAPIUrl(ctx context.Context, licenseKey string) (
func (ah *APIHandler) getIngestionUrlAndSigNozAPIUrl(ctx context.Context, licenseKey string) (
string, string, *basemodel.ApiError,
) {
url := fmt.Sprintf(
"%s%s",
strings.TrimSuffix(constants.ZeusURL, "/"),
"/v2/deployments/me",
)
// TODO: remove this struct from here
type deploymentResponse struct {
Status string `json:"status"`
Error string `json:"error"`
Data struct {
Name string `json:"name"`
ClusterInfo struct {
Region struct {
DNS string `json:"dns"`
} `json:"region"`
} `json:"cluster"`
} `json:"data"`
Name string `json:"name"`
ClusterInfo struct {
Region struct {
DNS string `json:"dns"`
} `json:"region"`
} `json:"cluster"`
}
resp, apiErr := requestAndParseResponse[deploymentResponse](
ctx, url, map[string]string{"X-Signoz-Cloud-Api-Key": licenseKey}, nil,
)
if apiErr != nil {
return "", "", basemodel.WrapApiError(
apiErr, "couldn't query for deployment info",
)
}
if resp.Status != "success" {
respBytes, err := ah.Signoz.Zeus.GetDeployment(ctx, licenseKey)
if err != nil {
return "", "", basemodel.InternalError(fmt.Errorf(
"couldn't query for deployment info: status: %s, error: %s",
resp.Status, resp.Error,
"couldn't query for deployment info: error: %w", err,
))
}
regionDns := resp.Data.ClusterInfo.Region.DNS
deploymentName := resp.Data.Name
resp := new(deploymentResponse)
err = json.Unmarshal(respBytes, resp)
if err != nil {
return "", "", basemodel.InternalError(fmt.Errorf(
"couldn't unmarshal deployment info response: error: %w", err,
))
}
regionDns := resp.ClusterInfo.Region.DNS
deploymentName := resp.Name
if len(regionDns) < 1 || len(deploymentName) < 1 {
// Fail early if actual response structure and expectation here ever diverge

View File

@@ -10,9 +10,6 @@ var SaasSegmentKey = GetOrDefaultEnv("SIGNOZ_SAAS_SEGMENT_KEY", "")
var FetchFeatures = GetOrDefaultEnv("FETCH_FEATURES", "false")
var ZeusFeaturesURL = GetOrDefaultEnv("ZEUS_FEATURES_URL", "ZeusFeaturesURL")
// this is set via build time variable
var ZeusURL = "https://api.signoz.cloud"
func GetOrDefaultEnv(key string, fallback string) string {
v := os.Getenv(key)
if len(v) == 0 {

View File

@@ -374,18 +374,12 @@ func (module *Module) GetOrCreateUser(ctx context.Context, user *types.User, opt
return existingUser, nil
}
newUser, err := types.NewUser(user.DisplayName, user.Email, user.Role, user.OrgID)
err = module.CreateUser(ctx, user, opts...)
if err != nil {
return nil, err
}
err = module.CreateUser(ctx, newUser, opts...)
if err != nil {
return nil, err
}
return newUser, nil
return user, nil
}
func (m *Module) CreateAPIKey(ctx context.Context, apiKey *types.StorableAPIKey) error {

View File

@@ -74,6 +74,63 @@ def zeus(
)
@pytest.fixture(name="gateway", scope="package")
def gateway(
network: Network,
request: pytest.FixtureRequest,
pytestconfig: pytest.Config,
) -> types.TestContainerDocker:
"""
Package-scoped fixture for running gateway
"""
def create() -> types.TestContainerDocker:
container = WireMockContainer(image="wiremock/wiremock:2.35.1-1", secure=False)
container.with_exposed_ports(8080)
container.with_network(network)
container.start()
return types.TestContainerDocker(
id=container.get_wrapped_container().id,
host_configs={
"8080": types.TestContainerUrlConfig(
"http",
container.get_container_host_ip(),
container.get_exposed_port(8080),
)
},
container_configs={
"8080": types.TestContainerUrlConfig(
"http", container.get_wrapped_container().name, 8080
)
},
)
def delete(container: types.TestContainerDocker):
client = docker.from_env()
try:
client.containers.get(container_id=container.id).stop()
client.containers.get(container_id=container.id).remove(v=True)
except docker.errors.NotFound:
logger.info(
"Skipping removal of Gateway, Gateway(%s) not found. Maybe it was manually removed?",
{"id": container.id},
)
def restore(cache: dict) -> types.TestContainerDocker:
return types.TestContainerDocker.from_cache(cache)
return dev.wrap(
request,
pytestconfig,
"gateway",
lambda: types.TestContainerDocker(id="", host_configs={}, container_configs={}),
create,
delete,
restore,
)
@pytest.fixture(name="make_http_mocks", scope="function")
def make_http_mocks() -> Callable[[types.TestContainerDocker, List[Mapping]], None]:
def _make_http_mocks(

View File

@@ -20,6 +20,7 @@ logger = setup_logger(__name__)
def signoz( # pylint: disable=too-many-arguments,too-many-positional-arguments
network: Network,
zeus: types.TestContainerDocker,
gateway: types.TestContainerDocker,
sqlstore: types.TestContainerSQL,
clickhouse: types.TestContainerClickhouse,
request: pytest.FixtureRequest,
@@ -56,6 +57,7 @@ def signoz( # pylint: disable=too-many-arguments,too-many-positional-arguments
"SIGNOZ_WEB_DIRECTORY": "/root/web",
"SIGNOZ_INSTRUMENTATION_LOGS_LEVEL": "debug",
"SIGNOZ_PROMETHEUS_ACTIVE__QUERY__TRACKER_ENABLED": False,
"SIGNOZ_GATEWAY_URL": gateway.container_configs["8080"].base(),
}
| sqlstore.env
| clickhouse.env
@@ -121,6 +123,7 @@ def signoz( # pylint: disable=too-many-arguments,too-many-positional-arguments
sqlstore=sqlstore,
telemetrystore=clickhouse,
zeus=zeus,
gateway=gateway,
)
def delete(container: types.SigNoz) -> None:
@@ -141,6 +144,7 @@ def signoz( # pylint: disable=too-many-arguments,too-many-positional-arguments
sqlstore=sqlstore,
telemetrystore=clickhouse,
zeus=zeus,
gateway=gateway,
)
return dev.wrap(
@@ -156,6 +160,7 @@ def signoz( # pylint: disable=too-many-arguments,too-many-positional-arguments
sqlstore=sqlstore,
telemetrystore=clickhouse,
zeus=zeus,
gateway=gateway,
),
create=create,
delete=delete,

View File

@@ -127,12 +127,13 @@ class SigNoz:
sqlstore: TestContainerSQL
telemetrystore: TestContainerClickhouse
zeus: TestContainerDocker
gateway: TestContainerDocker
def __cache__(self) -> dict:
return self.self.__cache__()
def __log__(self) -> str:
return f"SigNoz(self={self.self.__log__()}, sqlstore={self.sqlstore.__log__()}, telemetrystore={self.telemetrystore.__log__()}, zeus={self.zeus.__log__()})"
return f"SigNoz(self={self.self.__log__()}, sqlstore={self.sqlstore.__log__()}, telemetrystore={self.telemetrystore.__log__()}, zeus={self.zeus.__log__()}, gateway={self.gateway.__log__()})"
@dataclass

View File

View File

@@ -0,0 +1,161 @@
from http import HTTPStatus
from typing import Callable
import requests
from wiremock.client import (
HttpMethods,
Mapping,
MappingRequest,
MappingResponse,
WireMockMatchers,
)
from fixtures import types
from fixtures.auth import add_license
from fixtures.logger import setup_logger
logger = setup_logger(__name__)
def test_generate_connection_params(
signoz: types.SigNoz,
create_user_admin: types.Operation, # pylint: disable=unused-argument
make_http_mocks: Callable[[types.TestContainerDocker, list], None],
get_token: Callable[[str, str], str],
) -> None:
"""Test to generate connection parameters for AWS SigNoz cloud integration."""
# Get authentication token for admin user
admin_token = get_token("admin@integration.test", "password123Z$")
add_license(signoz, make_http_mocks, get_token)
cloud_provider = "aws"
# Mock the deployment info query and ingestion key operations
make_http_mocks(
signoz.zeus,
[
Mapping(
request=MappingRequest(
method=HttpMethods.GET,
url="/v2/deployments/me",
headers={
"X-Signoz-Cloud-Api-Key": {
WireMockMatchers.EQUAL_TO: "secret-key"
}
},
),
response=MappingResponse(
status=200,
json_body={
"status": "success",
"data": {
"name": "test-deployment",
"cluster": {"region": {"dns": "test.signoz.cloud"}},
},
},
),
persistent=False,
)
],
)
make_http_mocks(
signoz.gateway,
[
# Mock the ingestion keys search endpoint
Mapping(
request=MappingRequest(
method=HttpMethods.GET,
url="/v1/workspaces/me/keys/search?name=aws-integration",
),
response=MappingResponse(
status=200,
json_body={"status": "success", "data": []},
),
persistent=False,
),
# Mock the ingestion key creation endpoint
Mapping(
request=MappingRequest(
method=HttpMethods.POST,
url="/v1/workspaces/me/keys",
json_body={
"name": "aws-integration",
"tags": ["integration", "aws"],
},
headers={
"X-Signoz-Cloud-Api-Key": {
WireMockMatchers.EQUAL_TO: "secret-key"
},
"X-Consumer-Username": {
WireMockMatchers.EQUAL_TO: "lid:00000000-0000-0000-0000-000000000000"
},
"X-Consumer-Groups": {WireMockMatchers.EQUAL_TO: "ns:default"},
},
),
response=MappingResponse(
status=200,
json_body={
"status": "success",
"data": {
"name": "aws-integration",
"value": "test-ingestion-key-123456",
},
"error": "",
},
),
persistent=False,
),
],
)
endpoint = f"/api/v1/cloud-integrations/{cloud_provider}/accounts/generate-connection-params"
response = requests.get(
signoz.self.host_configs["8080"].get(endpoint),
headers={"Authorization": f"Bearer {admin_token}"},
timeout=10,
)
# Assert successful response
assert (
response.status_code == HTTPStatus.OK
), f"Expected 200, got {response.status_code}: {response.text}"
# Parse response JSON
response_data = response.json()
# Assert response structure contains expected data
assert "data" in response_data, "Response should contain 'data' field"
# Assert required fields in the response data
expected_fields = [
"ingestion_url",
"ingestion_key",
"signoz_api_url",
"signoz_api_key",
]
for field in expected_fields:
assert (
field in response_data["data"]
), f"Response data should contain '{field}' field"
# Assert values for the returned fields
data = response_data["data"]
# ingestion_key is created by the mocked gateway and should match
assert data["ingestion_key"] == "test-ingestion-key-123456", (
"ingestion_key should match the mocked ingestion key"
)
# ingestion_url should be https://ingest.test.signoz.cloud based on the mocked deployment DNS
assert data["ingestion_url"] == "https://ingest.test.signoz.cloud", (
"ingestion_url should be https://ingest.test.signoz.cloud"
)
# signoz_api_url should be https://test-deployment.test.signoz.cloud based on the mocked deployment name and DNS
assert data["signoz_api_url"] == "https://test-deployment.test.signoz.cloud", (
"signoz_api_url should be https://test-deployment.test.signoz.cloud"
)