Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to the application description format for how parameters/properties are defined #6

Closed
Tracked by #20
phil-abb opened this issue May 30, 2024 · 3 comments · Fixed by #11
Closed
Tracked by #20
Assignees

Comments

@phil-abb
Copy link
Contributor

phil-abb commented May 30, 2024

This is a proposal for updates to how parameters/properties are defined in the application description document. The goal is to make this more flexible for future cases where some property values are coming via inputs from the customer but other property values are coming from other sources (e.g. the device) and not the customer.

  1. Using properties instead of parameters
    • This term feels a bit bit more generic in intent and adds some flexibility
  2. Updates to the targets
    • using pointer instead of key
    • using components instead of appliesTo
    • It may not be something we want to include with Margo but we could also have the ability to specify a prefix and/or suffix to append to the value provided for the parameter.
      • This could be handled in the helm chart itself but there may be cases where the helm chart isn't ideal and this would be helpful
  3. Splitting out the properties from the configuration section
    • This makes it more extensible because you're not tying the properties to what the customer has to provide since property values could potentially come from other sources.
    • There are a few cases where we might want to have defined properties that the customer would not provide values for. For example:
      • cluster specific configuration like
        • Storage Class Name
        • Node name
      • Environment variables the device has available
        • identity provider information
        • admin user information
    • For Margo we could define a set of device specific parameters the device must be able to provide
      • May not need this now but we shouldn't prevent it from being possible in the future
      • We shouldn't prevent someone from doing it outside of the Margo spec (meaning it would only work for their app on their device and not officially supported)
    • For Margo we could make it possible for devices to have environment variables defined that could be used
      • If the device has the environment variable it would be used
      • If the device doesn't have the environment variable the customer would be prompted for the value.
      • As part of our device definition we could include the environment variables the device has (if any) that are useable for this purpose
      • May not need this now but we shouldn't prevent it from being possible in the future
      • We shouldn't prevent someone from doing it outside of the Margo spec (meaning it would only work for their app on their device and not officially supported)
    • If this is something we want to support for V1 we will come back to defining what these replacement keys should be later and not part of this initial change to the schema.
  4. The sections and schema are pretty much the same just under a configuration section instead of combined with the properties

Simple Hello World Example

apiVersion: margo.org/v1-alpha1
kind: ApplicationDescription
metadata:
  id: northstartida.hello.world
  name: Hello World
  description: A basic hello world application
  version: 1.0
  catalog:
    application:
      icon: ./resources/hw-logo.png
      tagline: Northstar Industrial Application's hello world application.
      descriptionLong: ./resources/description.md
      releaseNotes: ./resources/release-notes.md
      licenseFile: ./resources/license.pdf
      site: http://www.northstar-ida.com
    author:
      - name: Roger Wilkershank
        email: [email protected]
    organization:
      - name: Northstar Industrial Applications
        site: http://northstar-ida.com
components:
  cluster:
    - name: hello-world
      type: helm.v3
      properties:  
        repository: oci://northstarida.azurecr.io/charts/hello-world
        revision: 1.0.1
        wait: true
properties:
  greeting:
    value: Hello
    targets:
    - pointer: /hello-world/env/APP_GREETING
  target:
    value: World
    targets:
    - pointer: /hello-world/env/APP_TARGET
configuration:
  sections:
    - name: General Settings
      settings:
        - property: greeting
          name: Greeting
          description: The greeting to use.
          dataType: text
          schema: requireText
        - property: target
          name: Greeting Target
          description: The target of the greeting.
          dataType: text
          schema: requireText
  schema:
    - name: requireText
      maxLength: 45
      allowEmpty: false

Complex Example

apiVersion: margo.org/v1-alpha1
kind: ApplicationDescription
metadata:
  id: northstartida.digitron.orchestrator
  name: Digitron orchestrator
  description: The Digitron orchestrator application
  version: 1.2.1 
  catalog:
    application:
      icon: ./resources/ndo-logo.png
      tagline: Northstar Industrial Application's next-gen, AI driven, Digitron instrument orchestrator.
      descriptionLong: ./resources/description.md
      releaseNotes: ./resources/release-notes.md
      licenseFile: ./resources/license.pdf
      site: http://www.northstar-ida.com
    author:
      - name: Roger Wilkershank
        email: [email protected]
    organization:
      - name: Northstar Industrial Applications
        site: http://northstar-ida.com
components:
  cluster:
    - name: digitron-orchestrator
      type: helm.v3
      properties:
        repository: oci://northstarida.azurecr.io/charts/northstarida-digitron-orchestrator
        revision: 1.0.9
        wait: true
    - name: database-services
      type: helm.v3
      properties: 
        repository: oci://quay.io/charts/realtime-database-services
        revision: 2.3.7
        wait: true
  stand-alone:
  - name: digitron-orchestrator-docker
    type: docker-compose
    properties:
      packageLocation: https://northsitarida.com/digitron/docker/digitron-orchestrator.tar.gz
      keyLocation: https://northsitarida.com/digitron/docker/public-key.asc
properties:
  storageClass:
    value: ${cluster.storageClass}
    targets:
      - pointer: /global/storageClassName
        components: ["digitron-orchestrator", "database-services"]
  hostname:
    value: ${node.hostname}
    targets:
      - pointer: /edge_url
        prefix: https://
        components: ["digitron-orchestrator"]
      - pointer: /global/ingress/tlsSecretName
        components: ["digitron-orchestrator", "database-services"]
      - pointer: /global/ingress/host
        components: ["digitron-orchestrator", "database-services"]
      - pointer: ENV.HOSTNAME
        components: ["digitron-orchestrator-docker"]
  idpName:
    value: ${idp.name}
    targets:
      - pointer: /idp/name
        components: ["digitron-orchestrator"]
      - pointer: ENV.IDP_NAME
        components: ["digitron-orchestrator-docker"]
  idpProvider:
    value: ${idp.provider}
    targets:
      - pointer: /idp/provider
        components: ["digitron-orchestrator"]
      - pointer: ENV.IDP_PROVIDER
        components: ["digitron-orchestrator-docker"]
  idpClientId:
    value: ${idp.clientId}
    targets:
      - pointer: /idp/clientId
        components: ["digitron-orchestrator"]
      - pointer: ENV.IDP_CLIENT_ID
        components: ["digitron-orchestrator-docker"]
  idpUrl:
    value: ${idp.url}
    targets:
      - pointer: /idp/providerUrl
        components: ["digitron-orchestrator"]
      - pointer: /idp/providerMetadata
        components: ["digitron-orchestrator"]
        suffix: /.well-known/openid-configuration
      - pointer: ENV.IDP_URL
        components: ["digitron-orchestrator-docker"]
  adminName:
    value: ${admin.fullname}
    targets:
      - pointer: /administrator/name
        components: ["digitron-orchestrator"]
      - pointer: ENV.ADMIN_NAME
        components: ["digitron-orchestrator-docker"]
  adminUpn:
    value: ${admin.username}
    targets:
      - pointer: /administrator/userPrincipalName
        components: ["digitron-orchestrator"]
      - pointer: ENV.ADMIN_USERNAME
        components: ["digitron-orchestrator-docker"]
  pollFrequency:
    value: 30
    targets: 
      - pointer: /settings/pollFrequency
        components: ["digitron-orchestrator"]
      - pointer: ENV.POLL_FREQUENCY
        components: ["digitron-orchestrator-docker"]
  siteId:
    targets:
      - pointer: /settings/siteId
        components: ["digitron-orchestrator"]
      - pointer: ENV.SITE_ID
        components: ["digitron-orchestrator-docker"]
  cpuLimit:
    targets:
      - pointer: /settings/limits/cpu
        components: ["digitron-orchestrator"]
  memoryLimit:
    targets:
      - pointer: /settings/limits/memory
        components: ["digitron-orchestrator"]
configuration:
  sections:
    - name: General
      settings:
        - property: pollFrequency
          name: Poll Frequency
          description: How often the service polls for updated data in seconds
          dataType: integer
          schema: pollRange
        - property: siteId
          name: Site Id
          description: Special identifier for the site (optional)
          dataType: string
          schema: optionalText
    - name: Identity Provider
      settings:
        - property: idpName
          name: Name
          description: The name of the Identity Provider to use
          dataType: string
          immutable: true
          schema: requiredText
        - property: idpProvider
          name: Provider
          description: Provider something something
          dataType: string
          immutable: true
          schema: requiredText
        - property: idpClientId
          name: Client ID
          description: The client id
          dataType: string
          immutable: true
          schema: requiredText
        - property: idpUrl
          name: Provider URL
          description: The url of the Identity Provider
          immutable: true
          dataType: string
          schema: url
    - name: Administrator
      settings:
        - property: adminName
          name: Presentation Name
          description: The presentation name of the administrator
          dataType: string
          schema: requiredText
        - property: adminUpn
          name: Principal Name
          description: The principal name of the administrator
          dataType: string
          schema: email
    - name: Resource Limits
      settings:
        - property: cpuLimit
          name: CPU Limit
          description: Maximum number of CPU cores to allow the application to consume
          dataType: double
          schema: cpuRange
        - property: memoryLimit
          name: Memory Limit
          description: Maximum number of memory to allow the application to consume
          dataType: integer
          schema: memoryRange
  schema:
    - name: requiredText
      maxLength: 45
      allowEmpty: false
    - name: email
      allowEmpty: false
      regexMatch: .*@[a-z0-9.-]*
    - name: url
      allowEmpty: false
      regexMatch: ^(http(s):\/\/.)[-a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,6}\b([-a-zA-Z0-9@:%_\+.~#?&//=]*)$
    - name: pollRange
      minValue: 30
      maxValue: 360
      allowEmpty: false
    - name: optionalText  
      minLength: 5
      allowEmpty: true
    - name: cpuRange
      minValue: 0.5
      maxPrecision: 1
      allowEmpty: false
    - name: memoryRange
      minValue: 16384
      allowEmpty: false
@phil-abb
Copy link
Contributor Author

Original post from @Haishi2016

It's unclear to me the relationship between properties and configurations. It seems a configuration refers to a property. If they are linked, what's the point of separating them out? I think "properties" already represent something that is "configurable"?

We'll be discussing that part of the proposal more next week (it will be a different PR). The difference is what is under the configuration section is information for the WOS to be able to provide a UI enabling a customer to provide any required/optional inputs. I'm aware of some use cases where the values for these parameters would be provided from places and not shown in the UI. The idea is to make the parameters section more extensible in the future and not to tie all parameters to needing a UI element (configuration).

@phil-abb
Copy link
Contributor Author

Original post from @Haishi2016

There a few places where the ${} syntax is used. We need to clarify on the exact syntax rules of such expressions - such as do we allow mixture of these expressions with constants, do we anticipated recursive expressions, and where these expressions can be sued.

Correct. What I have above are just some examples. If Margo decides to adopt the idea of allowing parameter values to come from other sources (e.g., the device) we'll need to define the expected format and what these keys/IDs are. This may not be something we want to adopt for V1 but I do have some use cases for it.

I guess the pointers point to specific paths in Helm values file. This format doesn't natively work for Docker compose or other artifact formats that might get introduced in the future. Maybe we shouldn't specify the exact injection path (which also makes maintenance hard) but use a Margo-specific templating stage to allow declared properties/configurations to be injected into corresponding artifacts through "margo expressions" in the artifacts.

This is just a format we were using internally since it was based on a specification. I think it could be parsed and used in other places besides helm charts but there might be a better way of doing it.

@Haishi2016 Can you give an example of what you're talking about? I'm not sure if I follow exactly what you mean.

@phil-abb
Copy link
Contributor Author

phil-abb commented Jun 5, 2024

@Haishi2016 in your other post you raised a question about the use of dataType

In the parameter section, schema and dataType are two separate fields. Propose to merge them into one schema field to reduce possible errors that a user choose a schema that is incompatible with a dataType - such as "maxLength" won't make sense for a boolean value.

I don't have a problem with moving the dataType to the schema because of the reason you state. However, this would indicate all settings would need to have a schema even if there are no validation rules. This may not be a big deal but it is an impact to think about. This trade-off might be worth it to make the schema rules more consistent.

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 a pull request may close this issue.

1 participant