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

Only release what is (well) tested and Test (well) everything we release #97

Open
2 tasks
tdaly61 opened this issue Feb 6, 2023 · 2 comments
Open
2 tasks
Assignees

Comments

@tdaly61
Copy link

tdaly61 commented Feb 6, 2023

Request Summary:

Proposal
The DA should adopt and endorse a design policy regarding all core Mojaloop code and artifacts that "we will only release code or artifacts in a Mojaloop release that are well tested AND we will to the very best of our ability remove all non well tested code and artifacts from any future releases."

Ideally In conjunction with this design policy decision, there should be a high priority initial investigation undertaken to examine the current code base and remove as much unused and untested code and artifacts as possible in the short term.

Request Details:

This request is focused on Pillar #4 (quality product) and works in conjunction with DA #93 and DA 96

Definitions:

  • code: any code in the Mojaloop helm chart (release) including the Mojaloop created containers that are used and installed into kubernetes as part of that release.
  • artifacts : helm charts, yaml files , json files anything that supports the packaging , deployment and running of the code that is part of the helm package or the Mojaloop containers.

Background:
Many may be un-aware that we have code and artifacts that are not at all tested or not well tested that we are releasing and providing as part of current Mojaloop releases and this means that any user who deploys a Mojaloop release is at risk of exposing failures and this risks users :

  1. experiencing a loss of confidence in Mojaloop's quality possibly damaging Mojaloop reputation
  2. users having to spend time (and therefore maybe money) understanding and working around failures
  3. dropping their Mojaloop investigations

Also this chews up time for Mojaloop developers who will need to spend bulk time interacting and assisting users with issues that could have been avoided.

A final signficant issue here is that inclusion of untested code increases (unnecessarily) the risk of security violations as if nothing else it increases the attack surface of our application(s)

Example(s):

  1. Ingres.
    there is a current thread of 44 replies on help-mojaloop with Dushime , where Miguel is providing copious assistance in his usual helpful and attentive fashion around Ingress endpoints for the standard helm installation of Mojaloop. Yet as developers working on the helm packaging we are shipping Ingres artifacts knowingly untested (see further refine ingress subsequent to updating to networking/v1 project#2987 ) [This one certainly does have security implications]

  2. kube-system (NTPD)
    this component is not deployed , nor used in the current releases and it is unclear that it has even been tested. If nothing else it is "bloat" increasing package sizes and increasing security "attack surface"

  3. Bof/Version v15.0.0 (added post v15.0.0 release)
    On trying to deploy the Business Operations Framework(Bof), it became questionable as to if the Bof had been tested with v15.0.0 and it appears likely that it was only tested with IaC and the adequacy and effectiveness of that is largely inaccessible to the rest of the community.

These examples raise the question of what else is in the code base that is untested and should be removed.

Impact :

  • adopting this design proposal may have an impact on delivery dates for our next release BUT the positive impact to quality and security make this a positive investment in meeting future release dates.

  • better user experience and confidence

  • avoid reputational damage / build reputation for quality

  • save $$ on post-release support/hand-holding

  • Deadline: as soon as possible i.e. what are we going to ship in Mojaloop v15

  • Impact (Teams): core teams mostly , including those of us working on helm/kubernetes/install

  • Impact (Components): all code and artifacts

Accountability:

  • Owner: Tom Daly
  • Raised By: Tom Daly

Decision(s):

  • Approved By:

Details

  • Actual decision made as a result of discussion

Follow-up:

  • Actions to implement the decisions
@mdebarros
Copy link
Member

mdebarros commented Feb 7, 2023

A couple of comments here...

  • Providing an option to expose an ingress is a standard Helm practice. ALTHOUGH, Most if not ALL Mojaloop's Ingress's are disabled in a Mojaloop Packaged release by default except for the services exposing the Mojaloop Family of APIs via a default/example DNS name such as {{service}}.local, and these are done purely for convenience.

    Here is a list of Ingress's that are enabled by default to my knowledge:

    • Account-lookup-service (i.e. FSPIOP Discovery API)
    • Quoting-service (i.e. FSPIOP Quoting API)
    • ML-API-Adapter (i.e. FSPIOP Transfer API)
    • Central-Settlements (i.e. Settlements API)
    • Central-Ledger Service (i.e. Admin API)
    • Testing-Toolkit UI / API (The only exception to the above rule)

    More info on this below as part of the first example.

  • As for the examples:

1. Ingres.
   there is a current thread of  44 replies on help-mojaloop with Dushime , where Miguel is providing copious assistance in his usual helpful and attentive fashion around Ingress endpoints for the standard helm installation of Mojaloop.  Yet as developers working on the helm packaging  we are shipping Ingres artifacts knowingly untested (see [further refine ingress subsequent to updating to networking/v1 project#2987](https://github.com/mojaloop/project/issues/2987) ) [This one certainly does have security implications]

☝️ A couple of points here:

  1. These "unused" ingress's are not enabled by default, and thus I do not see this being a "security" concern. They are provided as an option to deployers to enable them through a custom config if they desire so. Additionally, these APIs don't really expose anything other than metrics or health points, which a deployer may wish to expose to an external ingestion service....although there are other ways to do this more elegantly.
    I would also raise that if security is a concern here, then ALL API interactions should be exposed/controlled through an API Gateway where "security", "throttling" and other similiarly API related policies can be properly enforced, instead of through a standard Ingress. The means to expose a service through Kubernetes is really something that is up for the Deployer to decide on based on their requirements. Or they may choose to use something like the IaC to deploy Mojaloop which auto-magically includes an enterprise grade API Gateways that is secured through TLS, OAuth in conjunction with secured VPNs/Allow-lists, etc.
  2. I have never had any "support" questions or concerns around any of the ingress's that are disabled by default. I have only had to deal with issues related to the the Ingress services that are exposing one of the Mojaloop Family of APIs services which are enabled by default.
2. kube-system (NTPD)
   this component is not deployed , nor used in the current releases and it is unclear that it has even been tested. If nothing else it is "bloat" increasing package sizes and increasing security "attack surface"

☝️ A couple of points here:

  1. This chart is unused at ths time, and can happily be removed. It was used at one stage, and it was tested on a very old version of Kubernetes.
  2. Its worth mentioning that this chart is a stand-alone Helm chart and it is NOT part/packaged as part of a Mojaloop Packaged Release. I.e. Yes it is available through the Mojaloop Helm repo, but it is NOT part of a Mojaloop deployment. Someone would have to explicitly install this chart using something like helm install mojaloop/kube-system as an example.
  3. We also do not have any documentation/guides that instruct anybody to install this chart.
  4. Here is a list of "deprecated" charts that are also unsused, and should similairly be cleaned-up:
    1. NTP <-- Not used, and the 2nd example provided in this issue.
    2. forensicloggingsidecar <-- very old component that is no longer being used
    3. centralenduserregistry <-- very old component that is no longer being used
    4. finance-portal* <-- I am not sure if anybody is still using this portal, but it certainly has not been tested with the latest releases of Mojaloop as it has been deprecated by the BoF reporting/finance portals.
    5. monitoring/* <-- this has not been maintained, and we should consider fixing it or instead deprecate it

@bushjames
Copy link

Current release process has appropriate practices in place to ensure no untested components are added to the official Mojaloop release; however, there are some older components still present in our HELM charts which do not meet our test standards and should be removed or managed appropriately e.g. with "known issues" in release notes. @elnyry-sam-k will add any such components to the list of known issues.

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

No branches or pull requests

4 participants