feat(core): support AICORE_SERVICE_KEY env var for credentials and rename env vars

[yamaceay]

This PR aims to align the authentication conventions of core and gen packages more closely. Not directly applicable to base, since the definition of AI API client must remain unchanged. New unit tests are added + env vars are removed from unit tests.

[marikaner]

Just adding my 2 cents. As I am a python noob, @mwien please review ;)

[marikaner]

[pp] Not related to this PR, but possibly a nice new backlog item: I noticed that the variables are imported in a different order here. So this does not seem to be covered by linting. Possibly worth improving, if you think so too.

[ZhongpinWang]

First time reviewing python SDK :)

Some comments can be addressed later when doing the actual refactoring / cleaning up.

[ZhongpinWang]

[req] (maybe for the future) After struggling and looking into the codebase, I finally understand what is a vcap_key........ It is a JSON object access path starting from credentials stored in the form of a tuple...

I know it is not super relevant to this PR. But refactoring needed... And allow me to just put my thoughts here as a reference. I added this comment to the refactoring BLI.

I think we should really consider renaming few stuff.

• "Key" means a single property name
• "Access Path" means obj.credentials.clientid
• "Value" means the content of that key value pair, what we get when calling get(key)

Also it is pretty hard to understand what CredentialsValue actually means. We call the whole object inside VCAP_SERVICES['aicore'][0] a service binding.

And why do we define object schema in such a complicated way. Can we not just define the data class of the credentials object such as AiCoreCredentials? Then transform_fn takes an AI CORE service binding object and map it to AiCoreDestination? (I used the term Destination as it is pretty much the transformed object)

@mwien to give you an overview of how VCAP_SERVICES could actually look like:

{
	"VCAP_SERVICES": {
		"aicore": [
			{
				"label": "aicore",
				"provider": null,
				"plan": "<plan>",
				"name": "default_aicore",
				"tags": [],
				"instance_guid": "<some uuid>",
				"instance_name": "default_aicore",
				"binding_guid": "<some_other_uuid>",
				"binding_name": null,
				"credentials": {
					"serviceurls": {
						"AI_API_URL": "https://api.ai.......ml.hana.ondemand.com"
					},
					"appname": "<appname>",
					"clientid": "<clientid>",
					"clientsecret": "<clientsecret>",
					"identityzone": "<subdomain of the subaccount>",
					"identityzoneid": "<tenant id / subaccount id>",
					"url": "<auth_url>"
				},
				"syslog_drain_url": null,
				"volume_mounts": []
			}
		],
        // ...
    }
}

[marikaner]

I also got super confused by this, but I also believe that this might be because I am not used to python as much.

[ZhongpinWang]

[q/req] Is there anywhere documented that we should store .env in the root (for development I guess)? Or is it just your local setup, and you stored .env at the root?

Later we will have a sample server folder / package, which will be used for trying out purposes and we can put .env in that folder? cc @mwien

[yamaceay]

In the local setup, I ran into errors running the unit tests since .env interfered with the mock credentials. This was meant rather for local development, I will investigate further.

[ZhongpinWang]

I am just not entirely sure if we should or must put .env at root. Not sure what python colleagues previously used to do.

[yamaceay]

For reference: uv run pytest packages/core/tests without env-files override yields 10 errors in core package:

============================================================================ short test summary info =============================================================================
FAILED packages/core/tests/ai_core_client/test_ai_core_v2_client.py::TestAICoreV2Client::test_from_env - AssertionError: expected call not found.
...
FAILED packages/core/tests/ai_core_client/test_credentials.py::TestConfigHandling::test_fetch_credentials_from_vcap_services_with_x509_env_var - AssertionError: 'https://api.ai.<redacted>.ml.hana.ondemand.com/v2' != 'vcap-api-url/v2'
=================================================================== 10 failed, 71 passed, 3 warnings in 1.05s ====================================================================
make: *** [test] Error 1

[ZhongpinWang]

My idea was to not put .env locally for development at root. Then we don't need to exclude any env files.

.env will be put as part of the new sample server folder once Marcel finished creating it.

Or are we writing any .env in the workflow which causes the issue?

[marikaner]

is it not better to still exclude .env, though. so no one accidentally commits it if the have it?

I would also prefer to avoid scope creep on this PR.

[ZhongpinWang]

.gitignore already ignores the .env*. Marcel works on the sample code PR so there again we have the chance to define how we want to read the env file.

[ZhongpinWang]

Looks much better now! Thanks for your effort 👍

Changes applied.

[ZhongpinWang]

Suggested change

	Precedence order: kwargs > environment variables > service key > config file > VCAP service
	Precedence order: kwargs > separate environment variables > service key > config file > VCAP service

[pp] I find this comment a bit confusing. Service key is also a env var. Maybe you have better idea.