Skip to content

Commit

Permalink
Handle multiple backends for the TypeInstance (#657)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mateusz Kuziemko authored Mar 24, 2022
1 parent 5f348f9 commit 6b60379
Show file tree
Hide file tree
Showing 22 changed files with 327 additions and 195 deletions.
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
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
2 changes: 1 addition & 1 deletion hub-js/src/local/resolver/mutation/delete-type-instance.ts
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
2 changes: 1 addition & 1 deletion hub-js/src/local/resolver/mutation/lock-type-instances.ts
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

0 comments on commit 6b60379

Please sign in to comment.