j1-integration document: dedup relationships by sourceType, _class, and targetType

[ndowmon]

In the azure integration, two relationships with the same type are defined:

{
  _type: 'azure_security_group_rule',
  sourceType: 'azure_security_group',
  _class: RelationshipClass.ALLOWS,
  targetType: 'azure_subnet'
},
{
  _type: 'azure_security_group_rule',
  sourceType: 'azure_subnet',
  _class: RelationshipClass.ALLOWS,
  targetType: 'azure_security_group'
}

Seems like this should create two rows in documentation. Current deduplication for relationships is based on _type, so only one of these relationships will be documented. If we instead dedup by sourceType, _class, and targetType, we would be able to capture both of these relationships.

On the other hand, this may be an anti-pattern that we want to avoid (multiple relationships defined by a single _type).

[aiwilliams]

I don't think it's an anti-pattern to have a relationship type where instances have different entity types. The AWS integration produces aws_iam_role_assume_policy_trust relationships which related an AccessRole to an Account, or another AccessRole, or a Service.

I'd recommend taking a look at the docs for the AWS integration. The format there is a good example of how complex integrations may need a different output.

[ndowmon]

Thanks for the input @aiwilliams! I agree that it's not an anti-pattern. I do feel like deduping StepRelationshipMetadata by sourceType, _class, and targetType will avoid undesired behavior in j1-integration document commands in the future.

Perhaps there is some implication for StepEntityMetadata as well. For example, if the _type shows up multiple times, but the _class or resourceName properties differ, we could throw during j1-integration document or j1-integration collect (or somewhere else - whatever makes the most sense for the context).

[aiwilliams]

Perhaps there is some implication for StepEntityMetadata as well. For example, if the _type shows up multiple times, but the _class or resourceName properties differ, we could throw during j1-integration document or j1-integration collect (or somewhere else - whatever makes the most sense for the context).

AWS integration produces these relationships:

interface RuleProperties extends NetworkAclRuleProperties {
  _class: 'ALLOWS' | 'DENIES';
  _type: 'aws_network_acl_rule';
  region: AWSRegion;
  displayName: string;
}

That is, we'll need to support sourceType: _type: 'aws_network_acl_rule', _class: 'ALLOWS' AND _type: 'aws_network_acl_rule', _class: 'DENIES'.

Something to keep in mind: take a look at the way the AWS integration processes these rules today. The source entity depends on whether the rule is ingress or egress. When creating direct relationships, the sourceType will depend on how the integration wants the edge directed. When creating a mapped relationship, the sourceType will not change, but the direction can be specified to the mapper.

Are there implications of this for our metadata/documentation?

[ndowmon]

AWS integration produces these relationships:

interface RuleProperties extends NetworkAclRuleProperties {
  _class: 'ALLOWS' | 'DENIES';
  _type: 'aws_network_acl_rule';
  region: AWSRegion;
  displayName: string;
}

That is, we'll need to support sourceType: _type: 'aws_network_acl_rule', _class: 'ALLOWS' AND _type: 'aws_network_acl_rule', _class: 'DENIES'.

That seems to encounter essentially the same issue as the example. I think it's solvable by simply defining both relationships explicitly.

{
  _type: 'azure_security_group_rule',
  sourceType: 'azure_security_group',
  _class: RelationshipClass.ALLOWS,
  targetType: 'azure_subnet'
},
{
  _type: 'azure_security_group_rule',
  sourceType: 'azure_security_group',
  _class: RelationshipClass.DENIES,
  targetType: 'azure_subnet'
}

Something to keep in mind: take a look at the way the AWS integration processes these rules today. The source entity depends on whether the rule is ingress or egress. When creating direct relationships, the sourceType will depend on how the integration wants the edge directed. When creating a mapped relationship, the sourceType will not change, but the direction can be specified to the mapper.

Are there implications of this for our metadata/documentation?

Great question @aiwilliams. Again, I feel it's easiest to just explicitly define two instances of StepRelationshipMetadata, one for ingress and one for egress.

{
  _type: 'azure_security_group_rule',
  sourceType: 'azure_security_group',
  _class: RelationshipClass.ALLOWS,
  targetType: 'azure_subnet'
},
{
  _type: 'azure_security_group_rule',
  sourceType: 'azure_subnet',
  _class: RelationshipClass.ALLOWS,
  targetType: 'azure_security_group'
}

[aiwilliams]

Yup, those "two instances of metadata" examples make sense 👍

To clarify, I wanted to address this:

For example, if the _type shows up multiple times, but the _class or resourceName properties differ, we could throw

I don't think we can throw (an error, right?) if a _type appears multiple times in the array with different _class values, if that is what this meant.

[ndowmon]

@aiwilliams that comment is specifically about entities and not relationships. I'm not sure if we should throw or what, but I do believe it should be handled. I opened a separate issue to discuss: #316