Azure / azure-service-operator

Azure Service Operator allows you to create Azure resources using kubectl
https://azure.github.io/azure-service-operator/
MIT License
738 stars 194 forks source link

Bug: unable to set only workspaceResourceReference for ContainerGroup #4046

Closed t3mi closed 4 months ago

t3mi commented 4 months ago

Version of Azure Service Operator 2.7.0

Describe the bug According to the spec, it should be possible to reference existing Workspace resource using workspaceResourceReference for ContainerGroup but validation requires workspaceId and workspaceKey to be set.

To Reproduce

---
apiVersion: operationalinsights.azure.com/v1api20210601
kind: Workspace
metadata:
  name: sampleworkspace
  namespace: default
spec:
  location: westcentralus
  owner:
    name: aso-sample-rg
  sku:
    name: Standalone
---
apiVersion: containerinstance.azure.com/v1api20211001
kind: ContainerGroup
metadata:
  name: samplecontainergroup
  namespace: default
spec:
  location: westcentralus
  owner:
    name: aso-sample-rg
  containers:
    - name: samplecontainergroup
      image: mcr.microsoft.com/azuredocs/aci-helloworld
      ports:
        - protocol: TCP
          port: 80
      resources:
        requests:
          cpu: 1
          memoryInGB: 2
  diagnostics:
    logAnalytics:
      logType: ContainerInsights
      workspaceResourceReference:
        group: operationalinsights.azure.com
        kind: Workspace
        name: sampleworkspace
  osType: Linux
  restartPolicy: Always
  ipAddress:
    type: Public
    ports:
      - protocol: TCP
        port: 80

Expected behavior Container group to be successfully created

Actual behaviour error: error validating error validating data: [ValidationError(ContainerGroup.spec.diagnostics.logAnalytics): missing required field "workspaceId" in com.azure.containerinstance.v1api20211001.ContainerGroup.spec.diagnostics.logAnalytics, ValidationError(ContainerGroup.spec.diagnostics.logAnalytics): missing required field "workspaceKey" in com.azure.containerinstance.v1api20211001.ContainerGroup.spec.diagnostics.logAnalytics];

matthchr commented 4 months ago

Even in the latest REST API specification for the containerinstance API, those two fields are listed as required.

Can you link to the documentation that suggests you can avoid specifying them? From what I can tell ASO is behaving as expected here (and as documented in the service API specification).

matthchr commented 4 months ago

It looks to me like workspaceId and workspaceKey are required but maybe are automatically filled out by various portal solutions?

I think what we need here is:

  1. The ability to export the workspaceId from the Workspace into a configmap
  2. The ability to export the workspaceKey from the Workspace into a secret.
  3. The ability to import the workspaceId from a configMap on the ContainerGroup.

it would be helpful if you could confirm what tool you were using that avoided setting the workspaceId and workspaceKey so that we can confirm that it's just hiding the setting rather than actually allowing them to be unset at the API level.

t3mi commented 4 months ago

It seems I misunderstood the REST API contract - thought it's either resource reference or an id with key should be filled, just see no reason to specify resource reference if it's not used to populate needed values for better UX IMO. What've you suggested is nice and will greatly simplify the deployment chain automation so if you don't mind I'll close this "bug" and will open 2 feature requests - one per resource.

matthchr commented 4 months ago

You probably can just make 1 feature request and we can fix both in one go, since they tie together and from a testing perspective it might be nice to make use of the "flow" in a test as well.