Skip to content

Build a graph of resources. Use it to detect circular dependencies#1391

Merged
kddejong merged 10 commits intoaws-cloudformation:masterfrom
miparnisari:graph
Mar 7, 2020
Merged

Build a graph of resources. Use it to detect circular dependencies#1391
kddejong merged 10 commits intoaws-cloudformation:masterfrom
miparnisari:graph

Conversation

@miparnisari
Copy link
Copy Markdown
Contributor

@miparnisari miparnisari commented Feb 29, 2020

Changes:

While debugging I found it very useful to use networkx.drawing.nx_pydot.write_dot(self.graph, path) which creates a visual representation of the graph. I think it would be useful for customers as well. But I wasn't sure whether to include it in this PR because it needs another dependency, pydot and it would need some work to make it a fancy graph.

@PatMyron
Copy link
Copy Markdown
Contributor

PatMyron commented Feb 29, 2020

While debugging I found it very useful to use networkx.drawing.nx_pydot.write_dot(self.graph, path) which creates a visual representation of the graph. I think it would be useful for customers as well. But I wasn't sure whether to include it in this PR because it needs another dependency, pydot and it would need some work to make it a fancy graph.

Agreed, I think just documenting how to do that here is a nice compromise:

add networkx.drawing.nx_pydot.write_dot(self.graph, 'graph') to the end of __init__(self, cfn) in src/cfnlint/graph.py

cfn-python-lint $ pip3 install pydot       # install pydot
cfn-python-lint $ pip3 install -e . --user # install locally modified version of cfn-lint
cfn-python-lint $ cfn-lint template.yaml   # lint desired template
cfn-python-lint $ pbcopy < graph           # copy graph to paste somewhere like http://www.webgraphviz.com/ or https://sketchviz.com/

Screen Shot 2020-03-01 at 3 54 07 PM

Comment thread setup.py
Comment on lines +75 to +76
'networkx~=2.4;python_version>="3.5"',
'networkx~=2.1;python_version<"3.5"'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will need to vet new dependencies

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@PatMyron PatMyron changed the title Build a graph of resources. Use it to detect circular dependencies. F… Build a graph of resources. Use it to detect circular dependencies Mar 1, 2020
@PatMyron
Copy link
Copy Markdown
Contributor

PatMyron commented Mar 1, 2020

Nice! From testing, Fn::GetAtt's long form is the last YAML template dependency I could find that's still undetected:

  Resource5:
    Type: AWS::SNS::Topic
    Properties:
      DisplayName:
        Fn::GetAtt: Resource6.TopicName # dependency only caught in !GetAtt form
        # !GetAtt Resource6.TopicName
  Resource6:
    Type: AWS::SNS::Topic
    Properties:
      DisplayName:
        Ref: Resource5

Pushed to your branch if you want to use for testing

Comment thread src/cfnlint/graph.py Outdated
Comment thread src/cfnlint/graph.py Outdated
Comment thread src/cfnlint/graph.py Outdated
@benbridts
Copy link
Copy Markdown
Contributor

Agreed, I think just documenting how to do that here is a nice compromise:

Another option is to put it behind a command or option and test for the existence of pydot if that's used.

try:
    import pydot
except ImportError:
    # give some feedback here

@kddejong kddejong merged commit 6ef3d95 into aws-cloudformation:master Mar 7, 2020
@miparnisari miparnisari deleted the graph branch March 10, 2020 07:05
@miparnisari miparnisari mentioned this pull request Mar 10, 2020
3 tasks
svenstaro pushed a commit to archlinux/svntogit-community that referenced this pull request Jul 22, 2020
The new python-networkx dependency is from [1]

[1] aws-cloudformation/cfn-lint#1391

git-svn-id: file:///srv/repos/svn-community/svn@602273 9fca08f4-af9d-4005-b8df-a31f2cc04f65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rule E3004 is not checking for DependsOn declarations

4 participants