Azure / azure-cli

Azure Command-Line Interface
MIT License
3.98k stars 2.96k forks source link

New cmdlets for FlowLog feature #12187

Closed irrogozh closed 4 years ago

irrogozh commented 4 years ago

FlowLog feature was created around 3 years ago. It allows customer to view information about ingress and egress IP traffic through an NSG. It used to be a proxy child resource of NetworkWatcher. Customer could enable/disable it using POST ConfigureFlowLog call. The problem with this approach was that customers couldn’t deploy flowLogs through template and couldn’t easily track and update them.

So, it was decided to make flowLog a tracked ARM resource and add PUT/GET/DELETE operations.

Now we need to add corresponding cmdlets for CREATE/GET and DELETE

Parameters for CREATE cmdlet should be the same as for this cmdlet: https://docs.microsoft.com/en-us/cli/azure/network/watcher/flow-log?view=azure-cli-latest#az-network-watcher-flow-log-configure

Here is documentation on existing cmdlets (for POST calls): https://docs.microsoft.com/en-us/cli/azure/network/watcher/flow-log?view=azure-cli-latest

Swagger link: https://github.com/Azure/azure-rest-api-specs/blob/master/specification/network/resource-manager/Microsoft.Network/stable/2019-11-01/networkWatcher.json

Starting from: "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/networkWatchers/{networkWatcherName}/flowLogs/{flowLogName}": {

And here is an example for PUT operation: https://github.com/Azure/azure-rest-api-specs/blob/master/specification/network/resource-manager/Microsoft.Network/stable/2019-11-01/examples/NetworkWatcherFlowLogCreate.json

ETA

First half of March

Additional context

  1. One problem we encountered during discussion is what we should do about current az network watcher flow-log show cmdlet. Right now on the backend it makes a POST call and returns this object:
    {
    "targetResourceId": "/subscriptions/56abfbd6-ec72-4ce9-831f-bc2b6f2c5505/resourceGroups/cleanupservice/providers/Microsoft.Network/networkSecurityGroups/rg-cleanupservice-nsg5",
    "properties": {
    "storageId": "/subscriptions/56abfbd6-ec72-4ce9-831f-bc2b6f2c5505/resourceGroups/MyCanaryFlowLog/providers/Microsoft.Storage/storageAccounts/immutablelogfltestsa",
    "enabled": true,
    "retentionPolicy": {
      "days": 0,
      "enabled": false
    },
    "format": {
      "type": "JSON",
      "version": 1
    }
    },
    "flowAnalyticsConfiguration": {
    "networkWatcherFlowAnalyticsConfiguration": {
      "enabled": false,
      "workspaceId": "a9ea48c6-2184-419a-9e39-31a913279979",
      "workspaceRegion": "eastus",
      "workspaceResourceId": "/subscriptions/56abfbd6-ec72-4ce9-831f-bc2b6f2c5505/resourcegroups/flowlogsv2demo/providers/microsoft.operationalinsights/workspaces/hossamws",
      "trafficAnalyticsInterval": 60
    }
    }
    }

In a new cmdlet for GET call we need it to return

{
  "name": "Microsoft.Networkcleanupservicerg-cleanupservice-nsg5",
  "id": "/subscriptions/56abfbd6-ec72-4ce9-831f-bc2b6f2c5505/resourceGroups/NetworkWatcherRG/providers/Microsoft.Network/networkWatchers/NetworkWatcher_eastus2euap/FlowLogs/Microsoft.Networkcleanupservicerg-cleanupservice-nsg5",
  "etag": "W/\"149be128-02f3-4165-a8b2-3204792e425c\"",
  "properties": {
    "provisioningState": "Failed",
    "targetResourceId": "/subscriptions/56abfbd6-ec72-4ce9-831f-bc2b6f2c5505/resourceGroups/cleanupservice/providers/Microsoft.Network/networkSecurityGroups/rg-cleanupservice-nsg5",
    "targetResourceGuid": "fedd4848-4ce8-49ae-b308-824efdc42b14",
    "storageId": "/subscriptions/56abfbd6-ec72-4ce9-831f-bc2b6f2c5505/resourceGroups/MyCanaryFlowLog/providers/Microsoft.Storage/storageAccounts/immutablelogfltestsa",
    "enabled": true,
    "flowAnalyticsConfiguration": {
      "networkWatcherFlowAnalyticsConfiguration": {
        "enabled": false,
        "workspaceId": "a9ea48c6-2184-419a-9e39-31a913279979",
        "workspaceRegion": "eastus",
        "workspaceResourceId": "/subscriptions/56abfbd6-ec72-4ce9-831f-bc2b6f2c5505/resourcegroups/flowlogsv2demo/providers/microsoft.operationalinsights/workspaces/hossamws",
        "trafficAnalyticsInterval": 60
      }
    },
    "retentionPolicy": {
      "days": 0,
      "enabled": false
    },
    "format": {
      "type": "JSON",
      "version": 1
    }
  },
  "type": "Microsoft.Network/networkWatchers/FlowLogs",
  "location": "eastus2euap"
}
yonzhan commented 4 years ago

add to S166.

haroldrandom commented 4 years ago

@irrogozh Is there min API version restricted?

haroldrandom commented 4 years ago

@irrogozh Besides, I think it's possible to merge the response of GET call return into the previous response of POST call. It takes care of backward compatibility, meanwhile supporting new response content without breaking user experience.

image

What do you think?

irrogozh commented 4 years ago

@haroldrandom, it's a good idea as a temporary approach. But I don't think it's going to work in a long run. The reason is that

  1. Customer will not see essential properties of the resource. Like, provisioningState, for example. So, he will not be aware if creating of resource succeeded or not.
  2. Customer will not be able to use 'show' cmdlet, and, then, modify resource using 'update' cmdlet with this approach, right?

I start thinking that we will need 'update' cmdlet as well like it is done in CLI for other resources.


About your question on min API version. Corresponding API were added into swagger for 2019-11-01 version. But on the backend PUT/GET/DELETE calls will work for any early version.

haroldrandom commented 4 years ago

Sync decision from Teams discussion: we decided not to mess up the output of current show command (via HTTP POST call).

Instead, we decided to:

  1. add another option like --arm to let user explicitly give us instruction that they want new formatted output of what ARM returns (via HTTP GET call). The default value is ‘false’ for now.
  2. add deprecation/warning info to current show command to warn user that the current output format will be changed.
  3. also add deprecation/warning info to --arm option to warn user that the default value of this parameter will be changed from false to true. Such that user don't have to provide this parameter since show command will supports HTTP GET call only (ARM) by then.
  4. Add deprecation info to configure commands
  5. after 6 months, or service decide to finally retire old command before that time, we will apply those changes and remove deprecation info.

In this feature, we will implement new commands CREATE/DELETE/UPDATE and refactor SHOW command.

haroldrandom commented 4 years ago

Sorry, I found --arm is not necessary, new API GET call requires watcher_name and flow_log_name, that's already enough.