Azure / appcat-rulesets

Repository for maintaining Rulesets for Windup
Eclipse Public License 2.0
6 stars 8 forks source link

Refactors the existing Tomcat rule #61

Closed agoncal closed 1 year ago

agoncal commented 1 year ago

Agreed. It was based on a copy/paste of our migration guide. I've improved it. WDYT @karianna @brunoborges ?

showpune commented 1 year ago

The rule looks too general for me, for resource usage rules ( database, message, cache ), they contain two parts: 1) What is the product for backend service ( Oracle, MyQL, Redis...) 2) Check the cloud violation of https://12factor.net/config So what the rule is used for?

If we want to use the rule to check the config violation, I suggest different rule for different resource usage type ( db, message, kv, cache ). We needn't to distinguish tomcat or spring, just check connection parameter partner in xml/yaml/properties

Add a request https://github.com/Azure/windup-rulesets/pull/67 for discuss

agoncal commented 1 year ago

@showpune in the context.xml file, there is no easy way to detect which resource type it is. In type you can have whatever you want. To detect that this is a datasource pointing at a MySQL database, is quite cumbersome.

<Context>
  <Resource name="jdbc/TestDB" auth="Container"
    type="javax.sql.DataSource"
    driverClassName="com.mysql.jdbc.Driver"
    url="jdbc:mysql://localhost:3306/testdb"
    username="dbuser"
    password="dbpassword"
    maxTotal="20" maxIdle="10" maxWaitMillis="-1"/>
</Context>
showpune commented 1 year ago

May we know it is a database configuration from key words like jdbc, datasource? Add the context.xml check in https://github.com/Azure/windup-rulesets/pull/67

May I double clarify the purpose for this rule, like: 1) Detect customer are using a backend service -> Ask customer migrate the backend service into Azure: The reason is that in thin war, the library is possible not included in the war files, so the rule may not work to detect a backend service 2) Customer are using configuration in code and violate the cloud readiness config principal -> Ask customer to update the configuration from code

karianna commented 1 year ago

Agreed. It was based on a copy/paste of our migration guide. I've improved it. WDYT @karianna @brunoborges ?

Looks good!