From 2d06b9c689e72c56f2db4533404e5453b26e6c9c Mon Sep 17 00:00:00 2001 From: Arnaud Schoonjans Date: Fri, 19 Apr 2019 10:37:10 +0200 Subject: [PATCH 1/4] Remove the resource_type and the agent column from resource. --- src/inmanta/data.py | 66 +++++++++++++++++----------- src/inmanta/db/versions/v1.py | 3 -- src/inmanta/server/server.py | 2 +- tests/test_data.py | 81 ++++++++++++++++++++++++++++++++--- 4 files changed, 118 insertions(+), 34 deletions(-) diff --git a/src/inmanta/data.py b/src/inmanta/data.py index 0285a79c05..95e90d75a0 100644 --- a/src/inmanta/data.py +++ b/src/inmanta/data.py @@ -32,6 +32,7 @@ import pkgutil import inmanta.db.versions +from inmanta.resources import Id from inmanta import const import asyncpg @@ -1467,9 +1468,6 @@ class Resource(BaseDocument): resource_id = Field(field_type=str, required=True) resource_version_id = Field(field_type=str, required=True, part_of_primary_key=True) - resource_type = Field(field_type=str, required=True) - agent = Field(field_type=str, required=True) - # Field based on content from the resource actions last_deploy = Field(field_type=datetime.datetime) @@ -1483,6 +1481,20 @@ class Resource(BaseDocument): # the list contains full rv id's provides = Field(field_type=list, default=[]) # List of resource versions + @property + def agent(self): + return self._agent + + @property + def resource_type(self): + return self._resource_type + + def __init__(self, from_postgres=False, **kwargs): + super(Resource, self).__init__(from_postgres, **kwargs) + parsed_id = Id.parse_id(self.resource_version_id) + self._agent = parsed_id.agent_name + self._resource_type = parsed_id.entity_type + def make_hash(self): character = "|".join(sorted([str(k) + "||" + str(v) for k, v in self.attributes.items() if k not in ["requires", "provides", "version"]])) @@ -1594,9 +1606,10 @@ async def get_resources_report(cls, environment): else: deployed = latest + parsed_id = Id.parse_id(resource_id) result.append({"resource_id": resource_id, - "resource_type": latest["resource_type"], - "agent": latest["agent"], + "resource_type": parsed_id.entity_type, + "agent": parsed_id.agent_name, "latest_version": latest["model"], "deployed_version": deployed["model"] if "last_deploy" in deployed else None, "last_deploy": deployed["last_deploy"] if "last_deploy" in deployed else None}) @@ -1608,21 +1621,17 @@ async def get_resources_for_version(cls, environment, version, agent=None, - include_attributes=True, - no_obj=False, - include_undefined=True): - projection = "*" - if not include_attributes: - projection = ','.join(["id", "environment", "model", "resource_id", "resource_version_id", - "resource_type", "agent", "last_deploy", "status", "provides"]) + no_obj=False): + query = f"SELECT * FROM {Resource.table_name()} WHERE environment=$1 AND model=$2" + values = [cls._get_value(environment), cls._get_value(version)] + if agent: - (filter_statement, values) = cls._get_composed_filter(environment=environment, model=version, agent=agent) - else: - (filter_statement, values) = cls._get_composed_filter(environment=environment, model=version) - if not include_undefined: - filter_statement += " AND status NOT IN ($" + str(len(values) + 1) + ",$" + str(len(values) + 2) + ")" - values += [cls._get_value(const.ResourceState.undefined), cls._get_value(const.ResourceState.skipped_for_undefined)] - query = "SELECT " + projection + " FROM " + cls.table_name() + " WHERE " + filter_statement + query += f" AND resource_id LIKE $3" + # Escape characters which have a special meaning in a LIKE-based SQL regex + agent_escaped = agent.replace("_", "\\_").replace('%', '\\%') + regex = f"%[{agent_escaped},%=%]" + values.append(regex) + resources = [] async with cls._connection_pool.acquire() as con: async with con.transaction(): @@ -1631,6 +1640,9 @@ async def get_resources_for_version(cls, record = dict(record) record["attributes"] = json.loads(record["attributes"]) record["id"] = record["resource_version_id"] + parsed_id = Id.parse_id(record["resource_version_id"]) + record["agent"] = parsed_id.agent_name + record["resource_type"] = parsed_id.entity_type resources.append(record) else: resources.append(cls(from_postgres=True, **record)) @@ -1682,11 +1694,11 @@ async def get_with_state(cls, environment, version): @classmethod def new(cls, environment, resource_version_id, **kwargs): - from inmanta.resources import Id vid = Id.parse_id(resource_version_id) attr = dict(environment=environment, model=vid.version, resource_id=vid.resource_str(), - resource_version_id=resource_version_id, resource_type=vid.entity_type, agent=vid.agent_name) + resource_version_id=resource_version_id) + attr.update(kwargs) return cls(**attr) @@ -1788,6 +1800,8 @@ def to_dict(self): self.make_hash() dct = super(Resource, self).to_dict() dct["id"] = dct["resource_version_id"] + dct["agent"] = self._agent + dct["resource_type"] = self._resource_type return dct @@ -1938,13 +1952,15 @@ async def get_agents(cls, environment, version): Returns a list of all agents that have resources defined in this configuration model """ (filter_statement, values) = cls._get_composed_filter(environment=environment, model=version) - query = "SELECT DISTINCT agent FROM " + Resource.table_name() + " WHERE " + filter_statement - result = [] + query = "SELECT DISTINCT resource_id FROM " + Resource.table_name() + " WHERE " + filter_statement + result = set() async with cls._connection_pool.acquire() as con: async with con.transaction(): async for record in con.cursor(query, *values): - result.append(record["agent"]) - return result + resource_id = record["resource_id"] + agent_name = Id.parse_id(resource_id).agent_name + result.add(agent_name) + return list(result) @classmethod async def get_versions(cls, environment, start=0, limit=DBLIMIT): diff --git a/src/inmanta/db/versions/v1.py b/src/inmanta/db/versions/v1.py index 7687f573dd..a8b8196957 100644 --- a/src/inmanta/db/versions/v1.py +++ b/src/inmanta/db/versions/v1.py @@ -48,8 +48,6 @@ async def update(connection): model integer NOT NULL, resource_id varchar NOT NULL, resource_version_id varchar NOT NULL, - resource_type varchar NOT NULL, - agent varchar NOT NULL, last_deploy timestamp, attributes JSONB, attribute_hash varchar, @@ -59,7 +57,6 @@ async def update(connection): FOREIGN KEY (environment, model) REFERENCES configurationmodel (environment, version) ON DELETE CASCADE ); -CREATE INDEX resource_env_model_agent_index ON resource (environment, model, agent); CREATE INDEX resource_env_resourceid_index ON resource (environment, resource_id, model); CREATE UNIQUE INDEX resource_env_resourceversionid_index ON resource (environment, resource_version_id); diff --git a/src/inmanta/server/server.py b/src/inmanta/server/server.py index 6d9c358220..d46222ba9e 100644 --- a/src/inmanta/server/server.py +++ b/src/inmanta/server/server.py @@ -936,7 +936,7 @@ def get_version(self, env, version_id, include_logs=None, log_filter=None, limit if version is None: return 404, {"message": "The given configuration model does not exist yet."} - resources = yield data.Resource.get_resources_for_version(env.id, version_id, include_attributes=True, no_obj=True) + resources = yield data.Resource.get_resources_for_version(env.id, version_id, no_obj=True) if resources is None: return 404, {"message": "The given configuration model does not exist yet."} diff --git a/tests/test_data.py b/tests/test_data.py index 19eeed8a3f..f5df24ed4d 100644 --- a/tests/test_data.py +++ b/tests/test_data.py @@ -25,7 +25,6 @@ import types import pkgutil - from inmanta import data, const from inmanta.const import LogLevel from asyncpg import PostgresSyntaxError @@ -1048,7 +1047,7 @@ async def test_get_resources(init_dataclasses_and_load_schema): @pytest.mark.asyncio -async def test_get_resources_for_version(init_dataclasses_and_load_schema): +async def test_model_get_resources_for_version(init_dataclasses_and_load_schema): project = data.Project(name="test") await project.insert() @@ -1111,9 +1110,81 @@ async def make_with_status(i, status): assert len(resources) == 4 assert sorted([x.resource_version_id for x in resources]) == sorted([d, s, u, su]) - resources = await data.Resource.get_resources_for_version(env.id, 3, include_undefined=False) - assert len(resources) == 2 - assert sorted([x.resource_version_id for x in resources]) == sorted([d, s]) + +@pytest.mark.asyncio +async def test_model_get_resources_for_version_optional_args(init_dataclasses_and_load_schema): + project = data.Project(name="test") + await project.insert() + + env = data.Environment(name="dev", project=project.id, repo_url="", repo_branch="") + await env.insert() + + version = int(time.time()) + cm = data.ConfigurationModel(environment=env.id, version=version, date=datetime.datetime.now(), total=3, version_info={}) + await cm.insert() + + async def insert_resource(env_id, version, agent_name, path, status): + resource_version_id = f"std::File[{agent_name},path={path}],v={version}" + resource = data.Resource.new(environment=env_id, + resource_version_id=resource_version_id, + attributes={"path": path}, + status=status) + await resource.insert() + + await insert_resource(env.id, version, "agent1", "path1", const.ResourceState.deployed) + await insert_resource(env.id, version, "agent2", "path2", const.ResourceState.available) + await insert_resource(env.id, version, "agent1", "path3", const.ResourceState.undefined) + + result = await data.Resource.get_resources_for_version(env.id, version) + assert len(result) == 3 + assert sorted([r.agent for r in result]) == ["agent1", "agent1", "agent2"] + for r in result: + assert len(r.attributes) == 1 + + result = await data.Resource.get_resources_for_version(env.id, version, agent="agent2") + assert len(result) == 1 + assert result[0].agent == "agent2" + + result = await data.Resource.get_resources_for_version(env.id, version, no_obj=True) + assert len(result) == 3 + assert sorted([r["agent"] for r in result]) == ["agent1", "agent1", "agent2"] + for r in result: + assert len(r["attributes"]) == 1 + + +@pytest.mark.asyncio +async def test_model_get_resources_for_version_escaping(init_dataclasses_and_load_schema): + project = data.Project(name="test") + await project.insert() + + env = data.Environment(name="dev", project=project.id, repo_url="", repo_branch="") + await env.insert() + + version = int(time.time()) + cm = data.ConfigurationModel(environment=env.id, version=version, date=datetime.datetime.now(), total=5, version_info={}) + await cm.insert() + + async def insert_resource(env_id, version, agent_name, path, status): + resource_version_id = f"std::File[{agent_name},path={path}],v={version}" + resource = data.Resource.new(environment=env_id, + resource_version_id=resource_version_id, + attributes={"path": path}, + status=status) + await resource.insert() + + agent_names = ["agent1.local", + "agent12local", + "agent1_local", + "agent1%local", + "agent12345local"] + + for agent_name in agent_names: + await insert_resource(env.id, version, agent_name, "path1", const.ResourceState.deployed) + + for agent_name in agent_names: + result = await data.Resource.get_resources_for_version(env.id, version, agent=agent_name) + assert len(result) == 1 + assert result[0].agent == agent_name @pytest.mark.asyncio From 410d7e50e45fb0edfde1a90fe2fe011a0cd1cbcb Mon Sep 17 00:00:00 2001 From: Arnaud Schoonjans Date: Fri, 19 Apr 2019 10:55:42 +0200 Subject: [PATCH 2/4] Update changelog. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f5efd2e52..58607227c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ v 2019.2 Changes in this release: - Various bugfixes (#1046, #968, #1045) -- Migration from mongodb to postgres +- Migration from mongodb to postgres (#1023, #1024, #1025) - added metering using pyformance - Added influxdb reporter for protocol endpoint metrics - Remove the configuration option agent-run-at-start (#1055) From 0a9f25a0bf5f26a898451c7b9801e67a912afdae Mon Sep 17 00:00:00 2001 From: Arnaud Schoonjans Date: Fri, 19 Apr 2019 11:39:58 +0200 Subject: [PATCH 3/4] Add agent column. --- src/inmanta/data.py | 35 ++++++++++++----------------------- src/inmanta/db/versions/v1.py | 2 ++ 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/inmanta/data.py b/src/inmanta/data.py index 54ac5299a5..30a0218264 100644 --- a/src/inmanta/data.py +++ b/src/inmanta/data.py @@ -1468,6 +1468,8 @@ class Resource(BaseDocument): resource_id = Field(field_type=str, required=True) resource_version_id = Field(field_type=str, required=True, part_of_primary_key=True) + agent = Field(field_type=str, required=True) + # Field based on content from the resource actions last_deploy = Field(field_type=datetime.datetime) @@ -1481,10 +1483,6 @@ class Resource(BaseDocument): # the list contains full rv id's provides = Field(field_type=list, default=[]) # List of resource versions - @property - def agent(self): - return self._agent - @property def resource_type(self): return self._resource_type @@ -1492,7 +1490,6 @@ def resource_type(self): def __init__(self, from_postgres=False, **kwargs): super(Resource, self).__init__(from_postgres, **kwargs) parsed_id = Id.parse_id(self.resource_version_id) - self._agent = parsed_id.agent_name self._resource_type = parsed_id.entity_type def make_hash(self): @@ -1609,7 +1606,7 @@ async def get_resources_report(cls, environment): parsed_id = Id.parse_id(resource_id) result.append({"resource_id": resource_id, "resource_type": parsed_id.entity_type, - "agent": parsed_id.agent_name, + "agent": latest["agent"], "latest_version": latest["model"], "deployed_version": deployed["model"] if "last_deploy" in deployed else None, "last_deploy": deployed["last_deploy"] if "last_deploy" in deployed else None}) @@ -1622,16 +1619,12 @@ async def get_resources_for_version(cls, version, agent=None, no_obj=False): - query = f"SELECT * FROM {Resource.table_name()} WHERE environment=$1 AND model=$2" - values = [cls._get_value(environment), cls._get_value(version)] - if agent: - query += f" AND resource_id LIKE $3" - # Escape characters which have a special meaning in a LIKE-based SQL regex - agent_escaped = agent.replace("_", "\\_").replace('%', '\\%') - regex = f"%[{agent_escaped},%=%]" - values.append(regex) + (filter_statement, values) = cls._get_composed_filter(environment=environment, model=version, agent=agent) + else: + (filter_statement, values) = cls._get_composed_filter(environment=environment, model=version) + query = f"SELECT * FROM {Resource.table_name()} WHERE {filter_statement}" resources = [] async with cls._connection_pool.acquire() as con: async with con.transaction(): @@ -1641,7 +1634,6 @@ async def get_resources_for_version(cls, record["attributes"] = json.loads(record["attributes"]) record["id"] = record["resource_version_id"] parsed_id = Id.parse_id(record["resource_version_id"]) - record["agent"] = parsed_id.agent_name record["resource_type"] = parsed_id.entity_type resources.append(record) else: @@ -1697,7 +1689,7 @@ def new(cls, environment, resource_version_id, **kwargs): vid = Id.parse_id(resource_version_id) attr = dict(environment=environment, model=vid.version, resource_id=vid.resource_str(), - resource_version_id=resource_version_id) + resource_version_id=resource_version_id, agent=vid.agent_name) attr.update(kwargs) @@ -1800,7 +1792,6 @@ def to_dict(self): self.make_hash() dct = super(Resource, self).to_dict() dct["id"] = dct["resource_version_id"] - dct["agent"] = self._agent dct["resource_type"] = self._resource_type return dct @@ -1952,15 +1943,13 @@ async def get_agents(cls, environment, version): Returns a list of all agents that have resources defined in this configuration model """ (filter_statement, values) = cls._get_composed_filter(environment=environment, model=version) - query = "SELECT DISTINCT resource_id FROM " + Resource.table_name() + " WHERE " + filter_statement - result = set() + query = "SELECT DISTINCT agent FROM " + Resource.table_name() + " WHERE " + filter_statement + result = [] async with cls._connection_pool.acquire() as con: async with con.transaction(): async for record in con.cursor(query, *values): - resource_id = record["resource_id"] - agent_name = Id.parse_id(resource_id).agent_name - result.add(agent_name) - return list(result) + result.append(record["agent"]) + return result @classmethod async def get_versions(cls, environment, start=0, limit=DBLIMIT): diff --git a/src/inmanta/db/versions/v1.py b/src/inmanta/db/versions/v1.py index a8b8196957..424820f634 100644 --- a/src/inmanta/db/versions/v1.py +++ b/src/inmanta/db/versions/v1.py @@ -48,6 +48,7 @@ async def update(connection): model integer NOT NULL, resource_id varchar NOT NULL, resource_version_id varchar NOT NULL, + agent varchar NOT NULL, last_deploy timestamp, attributes JSONB, attribute_hash varchar, @@ -57,6 +58,7 @@ async def update(connection): FOREIGN KEY (environment, model) REFERENCES configurationmodel (environment, version) ON DELETE CASCADE ); +CREATE INDEX resource_env_model_agent_index ON resource (environment, model, agent); CREATE INDEX resource_env_resourceid_index ON resource (environment, resource_id, model); CREATE UNIQUE INDEX resource_env_resourceversionid_index ON resource (environment, resource_version_id); From d9768896ad493129442e318ce1e85f286b0341e0 Mon Sep 17 00:00:00 2001 From: Arnaud Schoonjans Date: Fri, 19 Apr 2019 11:44:40 +0200 Subject: [PATCH 4/4] removed test case the verifies correct escaping Fixes #1025 --- tests/test_data.py | 35 ----------------------------------- 1 file changed, 35 deletions(-) diff --git a/tests/test_data.py b/tests/test_data.py index f5df24ed4d..50145c19cf 100644 --- a/tests/test_data.py +++ b/tests/test_data.py @@ -1152,41 +1152,6 @@ async def insert_resource(env_id, version, agent_name, path, status): assert len(r["attributes"]) == 1 -@pytest.mark.asyncio -async def test_model_get_resources_for_version_escaping(init_dataclasses_and_load_schema): - project = data.Project(name="test") - await project.insert() - - env = data.Environment(name="dev", project=project.id, repo_url="", repo_branch="") - await env.insert() - - version = int(time.time()) - cm = data.ConfigurationModel(environment=env.id, version=version, date=datetime.datetime.now(), total=5, version_info={}) - await cm.insert() - - async def insert_resource(env_id, version, agent_name, path, status): - resource_version_id = f"std::File[{agent_name},path={path}],v={version}" - resource = data.Resource.new(environment=env_id, - resource_version_id=resource_version_id, - attributes={"path": path}, - status=status) - await resource.insert() - - agent_names = ["agent1.local", - "agent12local", - "agent1_local", - "agent1%local", - "agent12345local"] - - for agent_name in agent_names: - await insert_resource(env.id, version, agent_name, "path1", const.ResourceState.deployed) - - for agent_name in agent_names: - result = await data.Resource.get_resources_for_version(env.id, version, agent=agent_name) - assert len(result) == 1 - assert result[0].agent == agent_name - - @pytest.mark.asyncio async def test_escaped_resources(init_dataclasses_and_load_schema): project = data.Project(name="test")