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

Handle multiple backends for the TypeInstance #657

Merged
merged 13 commits into from
Mar 24, 2022
8 changes: 8 additions & 0 deletions hack/images/jinja2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ postgres
Prefix for db was removed. This is Jinja limitation. It shouldn't be a big problem as long
as there is no need to render the template twice with the same prefix.

### Configuration

There is a possibility of pre-processing data by setting options in the configuration file.
List of supported operations:
| Name | Default | Description |
| ------------------------- | ------------ | ---------------------------------------------------------------------------------------------------|
| prefix | "" | Adds a prefix to inputted data. The data will be accessible using the set prefix. |
| unpackValue | False | If the `value` prefix is set in the inputted data then the data will be unpacked from that prefix. |

## Prerequisites

Expand Down
15 changes: 12 additions & 3 deletions hack/images/jinja2/jinja2-cli/jinja2cli/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,15 +312,13 @@ def cli(opts, args, config): # noqa: C901

out = codecs.getwriter("utf8")(out)

if config.get("prefix") is not None and len(parsed_data) != 0:
parsed_data = {config["prefix"]: parsed_data}
parsed_data = preprocessing_data(config, parsed_data)

template_path = os.path.abspath(template_path)
out.write(render(template_path, parsed_data, extensions, opts.filters, opts.strict))
out.flush()
return 0


def parse_kv_string(pairs):
dict_ = {}
for pair in pairs:
Expand All @@ -329,6 +327,17 @@ def parse_kv_string(pairs):
return dict_


def preprocessing_data(config, data):
'''Return preprocessed data based on the applied configuration.'''

if config.get("unpackValue") is True and len(data) != 0 and "value" in data:
data = data.get("value", {})

if config.get("prefix") is not None and len(data) != 0:
data = {config["prefix"]: data}

return data

class LazyHelpOption(Option):
"An Option class that resolves help from a callable"

Expand Down
59 changes: 46 additions & 13 deletions hack/images/jinja2/jinja2-cli/tests/test_jinja2cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,76 +12,82 @@


@dataclass
class TestCase:
class RenderTestCase:
name: str
template: str
data: typing.Dict[str, typing.Any]
result: str

@dataclass
class PreprocessingDataTestCase:
name: str
config: typing.Dict[str, typing.Any]
data: typing.Dict[str, typing.Any]
result: typing.Dict[str, typing.Any]

render_testcases = [
TestCase(name="empty", template="", data={}, result=""),
TestCase(
RenderTestCase(name="empty", template="", data={}, result=""),
RenderTestCase(
name="simple",
template="<@ title @>",
data={"title": b"\xc3\xb8".decode("utf8")},
result=b"\xc3\xb8".decode("utf8"),
),
TestCase(
RenderTestCase(
name="prefix",
template="<@ input.key @>",
data={"input": {"key": "value"}},
result="value",
),
TestCase(
RenderTestCase(
name="two prefixes but one provided",
template="<@ input.key @>/<@ additionalinput.key @>",
data={"input": {"key": "value"}},
result="value/<@ additionalinput.key @>",
),
TestCase(
RenderTestCase(
name="missing prefix",
template="<@ input.key @>",
data={},
result="<@ input.key @>",
),
TestCase(
RenderTestCase(
name="items before attrs",
template="<@ input.values.key @>",
data={"input": {"values": {"key": "value"}}},
result="value",
),
TestCase(
RenderTestCase(
name="attrs still working",
template="<@ input.values() @>",
data={"input": {}},
result="dict_values([])",
),
TestCase(
RenderTestCase(
name="key with dot",
template="<@ input['foo.bar'] @>",
data={"input": {"foo.bar": "value"}},
result="value",
),
TestCase(
RenderTestCase(
name="missing key with dot",
template='<@ input["foo.bar"] @>',
data={},
result='<@ input["foo.bar"] @>',
),
TestCase(
RenderTestCase(
name="use default value",
template='<@ input["foo.bar"] | default("hello") @>',
data={},
result="hello",
),
TestCase(
RenderTestCase(
name="multiple dotted values",
template='<@ input.key.key["foo.bar/baz"] | default("hello") @>',
data={},
result="hello",
),
TestCase(
RenderTestCase(
name="multiline strings",
template="""<@ input.key.key["foo.bar/baz"] | default('hello
hello') @>""",
Expand All @@ -100,6 +106,33 @@ def test_render(tmp_path, case):
assert output == case.result


preprocessing_data_testcases = [
PreprocessingDataTestCase(
name="set prefix in the config should prefix the data",
config={"prefix": "testprefix"},
data = {"test": "test"},
result={"testprefix": {"test": "test"}}
),
PreprocessingDataTestCase(
name="set unpackValue in the config should remove the value prefix",
config={"unpackValue": True},
data = {"value": {"test": "test"}},
result={"test": "test"}
),
PreprocessingDataTestCase(
name="set unpackValue and prefix should output correct results",
config={"prefix": "testprefix", "unpackValue": True},
data = {"value": {"test": "test"}},
result={"testprefix": {"test": "test"}}
)
]

@pytest.mark.parametrize("case", preprocessing_data_testcases)
def test_preprocessing_data(case):
output = cli.preprocessing_data(case.config,case.data)
assert output == case.result


def test_random_password(tmp_path):
random_pass_path = tmp_path / "random.template"
random_pass_path.write_text("<@ random_password(length=4) @>")
Expand Down
3 changes: 1 addition & 2 deletions hack/images/merger/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@

This folder contains the Docker image which merges multiple input YAML files into a single one.

The Docker image contains the `merger.sh` helper script. The script is an entrypoint of the image, and it is used to prefix and merge all YAML files found in `$SRC` directory.
Each file is prefixed with a file name without extension.
The Docker image contains the `merger.sh` helper script. The script is an entrypoint of the image, and it is used to prefix and merge all YAML files found in `$SRC` directory. Additionally, if the YAML file contains the `value` key, then it is unpacked from that key. Each file is prefixed with a file name without extension.

## Installation

Expand Down
8 changes: 7 additions & 1 deletion hack/images/merger/merger.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,16 @@
SRC=${SRC:-"/yamls"}
OUT=${OUT:-"/merged.yaml"}

# prefix each file with its filename
for filename in "${SRC}"/*; do
filename=$(basename -- "$filename")
prefix="${filename%.*}"

# remove value key if exists
mkuziemko marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency reason, it would be good to also have unpackValue conf option here. When I went through our manifests, it was hard to understand why it works in this way, as this is a "hidden" functionality.

But let's do that on a new follow-up PR. It's not a blocker for merging this one 👍

if [[ $(yq e 'has("value")' "${SRC}"/"${filename}") == "true" ]]; then
yq e '.value' -i "${SRC}"/"${filename}"
fi

# prefix each file with its filename
yq e -i "{\"${prefix}\": . }" "${SRC}"/"${filename}"
done

Expand Down
2 changes: 1 addition & 1 deletion hub-js/graphql/local/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ type TypeInstanceResourceVersionSpec {
abstract: backendRef.abstract,
fetchInput: {
typeInstance: { resourceVersion: rev.resourceVersion, id: ti.id },
backend: { context: backendCtx.context, id: backendRef.id}
backend: { context: apoc.convert.fromJsonMap(backendCtx.context), id: backendRef.id}
}
} AS value
RETURN value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export async function deleteTypeInstance(

// NOTE: Need to be preserved with 'WITH' statement, otherwise we won't be able
// to access node's properties after 'DETACH DELETE' statement.
WITH *, {id: ti.id, backend: { id: backendRef.id, context: specBackend.context, abstract: backendRef.abstract}} as out
WITH *, {id: ti.id, backend: { id: backendRef.id, context: apoc.convert.fromJsonMap(specBackend.context), abstract: backendRef.abstract}} as out
DETACH DELETE ti, metadata, spec, tirs, specBackend

WITH *
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export async function getTypeInstanceStoredExternally(

WITH {
typeInstanceId: ti.id,
backend: { context: backendCtx.context, id: backendRef.id, abstract: backendRef.abstract}
backend: { context: apoc.convert.fromJsonMap(backendCtx.context), id: backendRef.id, abstract: backendRef.abstract}
} AS value
RETURN value
`,
Expand Down
51 changes: 2 additions & 49 deletions hub-js/src/local/storage/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ import {
import { JSONSchemaType } from "ajv/lib/types/json-schema";
import { TextEncoder } from "util";

// TODO(https://github.com/capactio/capact/issues/634):
// Represents the fake storage backend URL that should be ignored
// as the backend server is not deployed.
// It should be removed after a real backend is used in `test/e2e/action_test.go` scenarios.
export const FAKE_TEST_URL = "e2e-test-backend-mock-url:50051";

type StorageClient = Client<typeof StorageBackendDefinition>;

interface BackendContainer {
Expand Down Expand Up @@ -131,10 +125,6 @@ export default class DelegatedStorageService {
backendId: input.backend.id,
});
const backend = await this.getBackendContainer(input.backend.id);
if (!backend?.client) {
// TODO: remove after using a real backend in e2e tests.
continue;
}

const validateErr = this.validateInput(input, backend.validateSpec);
if (validateErr) {
Expand Down Expand Up @@ -179,11 +169,6 @@ export default class DelegatedStorageService {
backendId: input.backend.id,
});
const backend = await this.getBackendContainer(input.backend.id);
if (!backend?.client) {
// TODO(https://github.com/capactio/capact/issues/634): remove after using a real backend in e2e tests.
continue;
}

const validateErr = this.validateInput(input, backend.validateSpec);
if (validateErr) {
throw Error(
Expand Down Expand Up @@ -220,16 +205,6 @@ export default class DelegatedStorageService {
backendId: input.backend.id,
});
const backend = await this.getBackendContainer(input.backend.id);
if (!backend?.client) {
// TODO(https://github.com/capactio/capact/issues/634): remove after using a real backend in e2e tests.
result = {
...result,
[input.typeInstance.id]: {
key: input.backend.id,
},
};
continue;
}

const validateErr = this.validateInput(input, backend.validateSpec);
if (validateErr) {
Expand Down Expand Up @@ -274,10 +249,6 @@ export default class DelegatedStorageService {
backendId: input.backend.id,
});
const backend = await this.getBackendContainer(input.backend.id);
if (!backend?.client) {
// TODO(https://github.com/capactio/capact/issues/634): remove after using a real backend in e2e tests.
continue;
}

const validateErr = this.validateInput(input, backend.validateSpec);
if (validateErr) {
Expand Down Expand Up @@ -307,10 +278,6 @@ export default class DelegatedStorageService {
backendId: input.backend.id,
});
const backend = await this.getBackendContainer(input.backend.id);
if (!backend?.client) {
// TODO(https://github.com/capactio/capact/issues/634): remove after using a real backend in e2e tests.
continue;
}

const validateErr = this.validateInput(input, backend.validateSpec);
if (validateErr) {
Expand Down Expand Up @@ -341,10 +308,6 @@ export default class DelegatedStorageService {
backendId: input.backend.id,
});
const backend = await this.getBackendContainer(input.backend.id);
if (!backend?.client) {
// TODO(https://github.com/capactio/capact/issues/634): remove after using a real backend in e2e tests.
continue;
}

const validateErr = this.validateInput(input, backend.validateSpec);
if (validateErr) {
Expand Down Expand Up @@ -408,19 +371,9 @@ export default class DelegatedStorageService {
}
}

private async getBackendContainer(
id: string
): Promise<BackendContainer | undefined> {
private async getBackendContainer(id: string): Promise<BackendContainer> {
if (!this.registeredClients.has(id)) {
const spec = await this.storageInstanceDetailsFetcher(id);
if (spec.url === FAKE_TEST_URL) {
logger.debug(
"Skipping a real call as backend was classified as a fake one"
);
// TODO(https://github.com/capactio/capact/issues/634): remove after using a real backend in e2e tests.
return undefined;
}

logger.debug("Initialize gRPC BackendContainer", {
backend: id,
url: spec.url,
Expand Down Expand Up @@ -451,7 +404,7 @@ export default class DelegatedStorageService {
this.registeredClients.set(id, { client, validateSpec: storageSpec });
}

return this.registeredClients.get(id);
return this.registeredClients.get(id) as BackendContainer;
}

private static convertToJSONIfObject(val: unknown): string | undefined {
Expand Down
15 changes: 4 additions & 11 deletions internal/cli/testing/storage_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package testing

import (
"context"
"encoding/json"

"capact.io/capact/internal/logger"
"capact.io/capact/internal/ptr"
Expand Down Expand Up @@ -44,9 +43,9 @@ const testStorageTypeContextSchema = `
`

type typeInstanceValue struct {
URL string `json:"url"`
AcceptValue bool `json:"acceptValue"`
ContextSchema interface{} `json:"contextSchema"`
URL string `json:"url"`
AcceptValue bool `json:"acceptValue"`
ContextSchema string `json:"contextSchema"`
}

// StorageBackendRegister provides functionality to produce and upload test storage backend TypeInstance.
Expand Down Expand Up @@ -80,12 +79,6 @@ func NewStorageBackendRegister() (*StorageBackendRegister, error) {

// RegisterTypeInstances produces and uploads TypeInstances which describe Test storage backend.
func (i *StorageBackendRegister) RegisterTypeInstances(ctx context.Context) error {
var contextSchema interface{}
err := json.Unmarshal([]byte(testStorageTypeContextSchema), &contextSchema)
if err != nil {
return errors.Wrap(err, "while unmarshaling contextSchema")
}

in := &hublocalgraphql.CreateTypeInstanceInput{
CreatedBy: ptr.String("populator/test-storage-backend-registration"),
TypeRef: &hublocalgraphql.TypeInstanceTypeReferenceInput{
Expand All @@ -95,7 +88,7 @@ func (i *StorageBackendRegister) RegisterTypeInstances(ctx context.Context) erro
Value: typeInstanceValue{
URL: i.cfg.TestStorageBackendURL,
AcceptValue: true,
ContextSchema: contextSchema,
ContextSchema: testStorageTypeContextSchema,
},
}

Expand Down
Loading