AIS-1630 - Create Datasource API - #1294
Conversation
prachikurade1
commented
Aug 20, 2026
prachikurade1
commented
- Create Datasource API:
- Validates the request body - datasource name, provider id, metadata
- Metadata is validated as per the schema.json set for the provider
- Validation for duplicate datasource name
- Test the connection - return an error if it fails - network error, auth error or access error
- Insert if all the above passes and return the id of the datasource created.
Signed-off-by: Prachi Kurade <prachi.kurade1@ibm.com>
Signed-off-by: Prachi Kurade <prachi.kurade1@ibm.com>
Signed-off-by: Prachi Kurade <prachi.kurade1@ibm.com>
| ProviderID string `json:"provider_id" binding:"required"` | ||
| // Metadata holds the provider-specific configuration. Sensitive fields (format: "password" | ||
| // in the JSON schema) are encrypted at rest; all other fields are stored in plain text. | ||
| Metadata map[string]any `json:"metadata" binding:"required"` |
There was a problem hiding this comment.
Lets make this params similar to CreateApplication
| Name string `json:"name" binding:"required,min=3,max=100"` | ||
| // ProviderID identifies the provider implementation (e.g. "object_storage", "file_system"). |
There was a problem hiding this comment.
Can we also do other validations like supporting chars, case insensitive validations etc
| // providerObjectStorage is the provider ID for S3-compatible object storage connectors. | ||
| providerObjectStorage = "object_storage" | ||
| // providerFileSystem is the provider ID for SSH/SFTP file system connectors. | ||
| providerFileSystem = "file_system" |
There was a problem hiding this comment.
Let us move this to constant package as it might be used in multiple places
| existing, err := s.connectorRepo.GetByName(ctx, req.Name) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to check for existing connector: %w", err) | ||
| } |
There was a problem hiding this comment.
As mentioned above let us make this case insensitive
| // SensitiveFields returns the credential fields that must be encrypted at rest. | ||
| func (t *fileSystemTester) SensitiveFields() map[string]bool { | ||
| return map[string]bool{ | ||
| "private_key": true, | ||
| } | ||
| } |
There was a problem hiding this comment.
Let us not hardcode this. Let it be generic and be computed from values.schema.json whether its sensitive or not
| if !isAWS { | ||
| // IBM COS, MinIO, and other S3-compatible stores: supply the custom | ||
| // endpoint and enable path-style addressing. | ||
| o.BaseEndpoint = aws.String(endpointURL) | ||
| o.UsePathStyle = true | ||
| } |
There was a problem hiding this comment.
Let us make this inverse,
For AWS do the if check as it is specific and rest flow can be generic
| "github.com/aws/aws-sdk-go-v2/credentials" | ||
| "github.com/aws/aws-sdk-go-v2/service/s3" | ||
| "github.com/aws/smithy-go" |
There was a problem hiding this comment.
Is there not a common package which could provide common s3 methods?
or else we need to use aws-sdk is it?
There was a problem hiding this comment.
This package is compatible with s3 based storage systems and is the most recommended one.
| func regionFromEndpoint(endpointURL string) string { | ||
| cosAliases := map[string]string{"us": "us-south", "eu": "eu-de", "ap": "jp-tok"} |
There was a problem hiding this comment.
Again here is there not a generic way?
What if a region is removed/modified/deleted?
There was a problem hiding this comment.
Also I see default as "us-east-1". Was this discussed with the design team?
There was a problem hiding this comment.
This is the way even Digitize Service has implemented. A mapping needs to be maintained and updated if regions are changed.
Digitize mapping -
https://github.com/IBM/project-ai-services/blob/main/services/digitize/connectors/scanners/config.py#L48
"us-east-1" is the standard default region used for AWS. Checked Digitize implementation as well and they have done the same.
Default region - https://github.com/IBM/project-ai-services/blob/main/services/digitize/connectors/scanners/config.py#L36
| encryptionKey := os.Getenv("DB_ENCRYPTION_KEY") | ||
|
|
There was a problem hiding this comment.
Let us have constant for DB_ENCRYPTION_KEY in constants pkg
There was a problem hiding this comment.
Also, there should be a check for an empty string. I see you are checking during runtime, which isn't ideal. It should fail fast.
| secretKey, _ := params["secret_access_key"].(string) | ||
| prefix, _ := params["prefix"].(string) | ||
|
|
||
| isAWS := strings.Contains(endpointURL, awsEndpointSuffix) |
There was a problem hiding this comment.
What if user wants to use GCS or ABS, will this work?
Do we need to again check and add specific logics?
There was a problem hiding this comment.
For GCS it will work, will need minor tweaks to handle error codes for access/auth errors.
For Azure , it will not work because Azure Blob storage is not s3 compatible.
| if sensitiveKeys[k] { | ||
| plaintext, ok := v.(string) | ||
| if !ok { | ||
| return nil, fmt.Errorf("sensitive field %q must be a string", k) |
There was a problem hiding this comment.
This should be StatusBadRequest instead of server error (500).
Signed-off-by: Prachi Kurade <prachi.kurade1@ibm.com>
4 participants
